Don't try to resolve sidebar app URL when booting sidebar app

When attempting to create a build of the browser extension using the
local dev client and the production h service I encountered an issue where
resolving the `sidebarAppUrl` setting in `src/boot/index.js` failed.

This happens because the extension does not set this setting in the
sidebar app so the boot script fell back to trying to resolve the URL
template baked into the development client. This in turn failed
because `document.currentScript` is not set in this context.

The `sidebarAppUrl` setting is not actually needed when booting the sidebar app, so
this commit refactors the boot code to avoid generating it in this
context. This is done by moving the responsibility for determining which
part of the client to load from `boot/boot.js` into `boot/index.js` and
only resolving the necessary settings.

src/boot/boot.js

@@ -12,7 +12,14 @@ const commonPolyfills = [
 ];
 
 /**
- * @typedef Config
+ * @typedef SidebarAppConfig
+ * @prop {string} assetRoot - The root URL to which URLs in `manifest` are relative
+ * @prop {Object.<string,string>} manifest -
+ *   A mapping from canonical asset path to cache-busted asset path
+ */
+
+/**
+ * @typedef AnnotatorConfig
  * @prop {string} assetRoot - The root URL to which URLs in `manifest` are relative
  * @prop {string} sidebarAppUrl - The URL of the sidebar's HTML page
  * @prop {Object.<string,string>} manifest -
@@ -48,7 +55,7 @@ function injectScript(doc, src) {
 
 /**
  * @param {Document} doc
- * @param {Config} config
+ * @param {SidebarAppConfig|AnnotatorConfig} config
  * @param {string[]} assets
  */
 function injectAssets(doc, config, assets) {
@@ -74,9 +81,9 @@ function polyfillBundles(needed) {
  * This triggers loading of the necessary resources for the client
  *
  * @param {Document} doc
- * @param {Config} config
+ * @param {AnnotatorConfig} config
  */
-function bootHypothesisClient(doc, config) {
+export function bootHypothesisClient(doc, config) {
   // Detect presence of Hypothesis in the page
   const appLinkEl = doc.querySelector(
     'link[type="application/annotator+html"]'
@@ -120,9 +127,9 @@ function bootHypothesisClient(doc, config) {
  * Bootstrap the sidebar application which displays annotations.
  *
  * @param {Document} doc
- * @param {Config} config
+ * @param {SidebarAppConfig} config
  */
-function bootSidebarApp(doc, config) {
+export function bootSidebarApp(doc, config) {
   const polyfills = polyfillBundles(commonPolyfills);
 
   injectAssets(doc, config, [
@@ -140,18 +147,3 @@ function bootSidebarApp(doc, config) {
     'styles/sidebar.css',
   ]);
 }
-
-/**
- * Initialize the "sidebar" application if run in the sidebar's stub HTML
- * page or the "annotator" application otherwise.
- *
- * @param {Document} document_
- * @param {Config} config
- */
-export default function boot(document_, config) {
-  if (document_.querySelector('hypothesis-app')) {
-    bootSidebarApp(document_, config);
-  } else {
-    bootHypothesisClient(document_, config);
-  }
-}

src/boot/index.js

@@ -11,20 +11,26 @@
 
 import { parseJsonConfig } from './parse-json-config';
 
-import boot from './boot';
+import { bootHypothesisClient, bootSidebarApp } from './boot';
 import processUrlTemplate from './url-template';
 import { isBrowserSupported } from './browser-check';
 
 if (isBrowserSupported()) {
   const settings = parseJsonConfig(document);
-  boot(document, {
-    assetRoot: processUrlTemplate(settings.assetRoot || '__ASSET_ROOT__'),
-    // @ts-ignore - `__MANIFEST__` is injected by the build script
-    manifest: __MANIFEST__,
-    sidebarAppUrl: processUrlTemplate(
+  const assetRoot = processUrlTemplate(settings.assetRoot || '__ASSET_ROOT__');
+  // @ts-ignore - `__MANIFEST__` is injected by the build script
+  const manifest = __MANIFEST__;
+
+  // Check whether this is the sidebar app (indicated by the presence of a
+  // `<hypothesis-app>` element) and load the appropriate part of the client.
+  if (document.querySelector('hypothesis-app')) {
+    bootSidebarApp(document, { assetRoot, manifest });
+  } else {
+    const sidebarAppUrl = processUrlTemplate(
       settings.sidebarAppUrl || '__SIDEBAR_APP_URL__'
-    ),
-  });
+    );
+    bootHypothesisClient(document, { assetRoot, manifest, sidebarAppUrl });
+  }
 } else {
   // Show a "quiet" warning to avoid being disruptive on non-Hypothesis sites
   // that embed the client.

src/boot/test/boot-test.js

-import boot from '../boot';
+import { bootHypothesisClient, bootSidebarApp } from '../boot';
 import { $imports } from '../boot';
 
 function assetUrl(url) {
@@ -27,7 +27,7 @@ describe('bootstrap', function () {
     iframe.remove();
   });
 
-  function runBoot() {
+  function runBoot(app = 'annotator') {
     const assetNames = [
       // Polyfills
       'scripts/polyfills-es2017.bundle.js',
@@ -53,10 +53,17 @@ describe('bootstrap', function () {
       return manifest;
     }, {});
 
-    boot(iframe.contentDocument, {
+    let bootApp;
+    if (app === 'annotator') {
+      bootApp = bootHypothesisClient;
+    } else if (app === 'sidebar') {
+      bootApp = bootSidebarApp;
+    }
+
+    bootApp(iframe.contentDocument, {
       sidebarAppUrl: 'https://marginal.ly/app.html',
       assetRoot: 'https://marginal.ly/client/',
-      manifest: manifest,
+      manifest,
     });
   }
 
@@ -76,9 +83,9 @@ describe('bootstrap', function () {
     return scripts.concat(styles).sort();
   }
 
-  context('in the host page', function () {
+  describe('bootHypothesisClient', function () {
     it('loads assets for the annotation layer', function () {
-      runBoot();
+      runBoot('annotator');
       const expectedAssets = [
         'scripts/annotator.bundle.1234.js',
         'styles/annotator.1234.css',
@@ -89,7 +96,7 @@ describe('bootstrap', function () {
     });
 
     it('creates the link to the sidebar iframe', function () {
-      runBoot();
+      runBoot('annotator');
 
       const sidebarAppLink = iframe.contentDocument.querySelector(
         'link[type="application/annotator+html"]'
@@ -103,7 +110,7 @@ describe('bootstrap', function () {
       link.type = 'application/annotator+html';
       iframe.contentDocument.head.appendChild(link);
 
-      runBoot();
+      runBoot('annotator');
 
       assert.deepEqual(findAssets(iframe.contentDocument), []);
     });
@@ -113,7 +120,7 @@ describe('bootstrap', function () {
         sets.filter(s => s.match(/es2017/))
       );
 
-      runBoot();
+      runBoot('annotator');
 
       const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
         a.match(/polyfills/)
@@ -125,20 +132,9 @@ describe('bootstrap', function () {
     });
   });
 
-  context('in the sidebar application', function () {
-    let appRootElement;
-
-    beforeEach(function () {
-      appRootElement = iframe.contentDocument.createElement('hypothesis-app');
-      iframe.contentDocument.body.appendChild(appRootElement);
-    });
-
-    afterEach(function () {
-      appRootElement.remove();
-    });
-
+  describe('bootSidebarApp', function () {
     it('loads assets for the sidebar application', function () {
-      runBoot();
+      runBoot('sidebar');
       const expectedAssets = [
         'scripts/katex.bundle.1234.js',
         'scripts/sentry.bundle.1234.js',
@@ -156,7 +152,7 @@ describe('bootstrap', function () {
         sets.filter(s => s.match(/es2017/))
       );
 
-      runBoot();
+      runBoot('sidebar');
 
       const polyfillsLoaded = findAssets(iframe.contentDocument).filter(a =>
         a.match(/polyfills/)