1. 18 Oct, 2021 1 commit
  2. 15 Oct, 2021 14 commits
    • Robert Knight's avatar
      Convert test bundle to be an ES module · 53da4038
      Robert Knight authored
      This should make it easier to speed up test runs in future by
      pre-building npm dependencies into a separate bundle which the main test
      bundle can import from.
      53da4038
    • Robert Knight's avatar
      Limit CommonJS processing to npm dependencies · f72de5b5
      Robert Knight authored
      The CommonJS plugin is the second most expensive transform after Babel.
      Since all of our own code uses ES modules, limit it to npm dependencies
      to speed up the build.
      
      In tests the Babel transform has also been moved last so that additional
      code it generates is not unnecessarily processed by subsequent plugins.
      
      Combined these changes speed up a build of the full test bundle by ~4s.
      f72de5b5
    • Robert Knight's avatar
      Use an Error-like object rather than an actual Error in a test · f16678ec
      Robert Knight authored
      This fixes a slow test on this branch, which appears to be caused by an
      interaction between using `postMessage` to serialize native Error
      objects and the `source-map-support` Karma plugin that we use.
      
      Sending native Errors via `PortRPC` currently doesn't work in all
      browsers so I expect we'll want to implement our own serialization of
      errors in `PortRPC` to work around that. That can also work around the
      `source-map-support`/`postMessage` issue in Chrome in future.
      
      To move the Browserify => Rollup conversion forwards, this commit just
      changes the test to use an Error-like rather than actual Error object.
      f16678ec
    • Robert Knight's avatar
      Remove unnecessary sinon import · f83ad4af
      Robert Knight authored
      `sinon` is exposed as a global in the test environment so it doesn't
      need to be imported. Additionally some of the dependencies of Sinon
      (samsam, nise) cause warnings about circular dependencies and use of
      `eval` when bundled by Rollup.
      f83ad4af
    • Robert Knight's avatar
      Add some clarifying comments · fd99644f
      Robert Knight authored
      Add comments to clarify a few things about the way the tests are built
      and run.
      fd99644f
    • Robert Knight's avatar
      Build boot script as classic rather than module script · 8c92b413
      Robert Knight authored
      Due to pre-existing `<script>` tags on third-party websites that load
      the client's boot script, via https://hypothes.is/embed.js, we need to
      keep generating it as a classic rather than ES module script for now.
      8c92b413
    • Robert Knight's avatar
      Move launch error rendering into a function · 3185bf1b
      Robert Knight authored
      This prevents a warning from Rollup about use of `this` outside a class
      or (non-arrow) function in development builds, due to the way Babel
      compiles JSX expressions [1]. Moving top-level code into a function is
      also useful in case we ever want to write tests for it.
      
      [1] https://github.com/babel/babel/issues/9149
      3185bf1b
    • Robert Knight's avatar
      d65b65b3
    • Robert Knight's avatar
      Add note about `preact/debug` exclusion in prod builds · a6e071d7
      Robert Knight authored
      Since the exclusion is no longer triggered by logic in the source code,
      add a note about where this happens.
      a6e071d7
    • Robert Knight's avatar
      Use @rollup/plugin-virtual to exclude modules in production · 8743910d
      Robert Knight authored
      Make conditional exclusion of modules work the same way in the app
      bundle as in the tests bundle.
      8743910d
    • Robert Knight's avatar
      Fix boot script error when running `make dev` after `make clean` · e40f8438
      Robert Knight authored
      Fix an occasional error from Gulp about a missing `boot.bundle.js` file
      when running `make dev` for the first time after `make clean`. This can
      happen if `updateManifest` runs before Rollup has generated
      `boot.bundle.js`. The error can be safely ignored as `updateManifest`
      will be re-run after `boot.bundle.js` is generated.
      e40f8438
    • Robert Knight's avatar
    • Robert Knight's avatar
    • Robert Knight's avatar
      Replace Browserify with Rollup · bcf7ef0a
      Robert Knight authored
      Replace Browserify with Rollup as a module bundler. This brings several
      advantages:
      
      - It is a more modern bundler that has better support in the ecosystem,
        more active maintenence, fewer dependencies and is easier to write plugins for.
        The core team also maintain most of the plugins that we need.
      
      - It has native support for modern platform features like ES modules,
        including dynamic import for lazy-loading of functionality.
      
      - It generates smaller bundles due to its ability to do tree shaking and scope hoisting.
      
      The new bundles are generated as ES modules rather than UMD / IIFE bundles.
      Our target browsers now all support ES modules and this will enable us to use
      native static/dynamic imports for code splitting or lazy-loading of
      functionality in future.
      
      The initial Rollup build generates one bundle per application (boot script,
      annotator, sidebar). This simplifies the build process and minimizes production
      bundle size, at the cost of making incremental updates during development
      slightly slower.
      
      Build performance is currently similar to or a little slower than Browserify.
      We can optimize this by adding caching of transforms (mainly Babel) and
      pre-building of a vendor bundle containing large dependencies.
      
      As part of changing the bundler this also changes how the code is packaged
      for tests.  We now do the bundling ourselves rather than using a Karma
      integration. This makes it much easier to inspect the processed code and fix
      problems. It also makes it easier to optimize the bundle building in future.
      bcf7ef0a
  3. 11 Oct, 2021 14 commits
  4. 08 Oct, 2021 2 commits
    • Eduardo Sanz García's avatar
      Add documentation to test cases · 0356d203
      Eduardo Sanz García authored
      * add information about why to use `window.load` instead of
        `document.DOMContentLoaded` to check when a document is ready
      
      * use an invalid local URL (http://localhost:1) to speed DNS lookup in
        the test
      
      * include some comments about the use of `waitForFrameObserver` in the
        tests
      0356d203
    • Eduardo Sanz García's avatar
      Provide `externalContainerSelector` config in a different way · ba206357
      Eduardo Sanz García authored
      Instead of override the window.hypothesisConfig, I provide the
      `externalContainerSelector` through a JSON script tag. This allows a
      more incremental way to add configuration options.
      ba206357
  5. 07 Oct, 2021 9 commits
    • Robert Knight's avatar
    • Robert Knight's avatar
      Proxy client asset locations from host frame · fc314751
      Robert Knight authored
      When injecting the client into a guest frame, proxy any custom asset locations
      specified via `.js-hypothesis-config` script tags in the host frame.
      These are used by the browser extension and needed for the VitalSource
      integration to work in that context.
      
      We could probably avoid copying the `sidebarAppUrl` and `notebookAppUrl`
      settings for guest-only frames. This will require changes to the boot
      script and possibly the annotator entry point.
      fc314751
    • Robert Knight's avatar
      Revise HypothesisInjector class docs · 576b7355
      Robert Knight authored
      576b7355
    • Robert Knight's avatar
    • Robert Knight's avatar
      Implement `Guest.injectClient` · a8b50cf3
      Robert Knight authored
      Implement the `injectClient` method of Guest that is used by
      integrations (eg. VitalSource) to inject the client into a chosen frame.
      
      In the process the `HypothesisInjector` class has been extracted out of
      the `CrossFrame` class, since it is unrelated to the rest of the
      functionality in that class and only lived their because of its
      dependence on the `Bridge` instance, which will soon be removed (see
      https://github.com/hypothesis/client/pull/3812). It is now constructed
      and used directly by the `Guest` class instead.
      a8b50cf3
    • Robert Knight's avatar
      Add `injectClient` method to HypothesisInjector · 3cb14c36
      Robert Knight authored
      Expose the existing logic for injecting the client into a target frame
      in HypothesisInjector as a public `injectClient` method and modify it to
      wait for the document to be loaded if necessary. This method will be
      used by the guest class to inject the client into specific frames when
      requested by the current integration.
      
      In the case where the frame is discovered by `FrameObserver` this means
      that `onDocumentReady(frame)` will be called twice. That's OK because
      the second call will just complete immediately if the frame is already
      ready.
      3cb14c36
    • Robert Knight's avatar
      05b6a932
    • Robert Knight's avatar
      Route guest frame unload notifications via host frame · c56cf3fa
      Robert Knight authored
      Change how the sidebar is notified of guest frames being unloaded to
      support guest frames where the client has been loaded via means other
      than `HypothesisInjector` or where the guest is cross-origin.
      
      Instead of listening for the guest frame's 'unload' event from the
      parent frame in `HypothesisInjector`, the guest frame instead listens
      for this event itself and sends a `hypothesisGuestUnloaded` message to
      the host frame via `window.postMessage`, which in turn is handled in the
      `Sidebar` class to relay it to the sidebar app via a `destroyFrame` RPC
      call. This indirect route works around a bug in Safari (see code
      comments).
      
      As well as supporting future use cases, this also simplifies the
      `HypothesisInjector` class as it no longer needs access to the `Bridge`.
      c56cf3fa
    • Robert Knight's avatar
      Add toggle button to "Annotatable iframe" test page · 84444667
      Robert Knight authored
      This enables testing handling of the client's discovery of existing
      frames (on page load) as well as handling of dynamic addition and removal after
      the client has started.
      84444667