Move `jsonConfigsFrom` function from `src/shared` to `src/boot`

Code in the `boot/` module needs to be compiled to work in older
browsers than code shared between the annotator and sidebar applications
in `src/shared`. To ensure this it helps to avoid dependencies from
code in `src/boot` on code in `src/shared`, but the converse is OK.

This commit moves the `jsonConfigsFrom` function from `shared/settings`
to `boot/parse-json-config` so that it is compiled with the same
settings as the rest of the code in the `boot/` module and to reduce the
chances of someone accidentally introducing ES6+ features into this code
in future and forgetting about older browsers.

The function was also renamed to make it more obvious that it parses
JSON config, and thus may fail, rather than returning a JSON string.

src/annotator/config/settings.js

-import * as sharedSettings from '../../shared/settings';
+import { parseJsonConfig } from '../../boot/parse-json-config';
 
 import configFuncSettingsFrom from './config-func-settings-from';
 import isBrowserExtension from './is-browser-extension';
 
 export default function settingsFrom(window_) {
-  const jsonConfigs = sharedSettings.jsonConfigsFrom(window_.document);
+  const jsonConfigs = parseJsonConfig(window_.document);
   const configFuncSettings = configFuncSettingsFrom(window_);
 
   /**

src/annotator/config/test/settings-test.js

@@ -4,19 +4,17 @@ import { $imports } from '../settings';
 describe('annotator.config.settingsFrom', function () {
   let fakeConfigFuncSettingsFrom;
   let fakeIsBrowserExtension;
-  let fakeSharedSettings;
+  let fakeParseJsonConfig;
 
   beforeEach(() => {
     fakeConfigFuncSettingsFrom = sinon.stub().returns({});
     fakeIsBrowserExtension = sinon.stub().returns(false);
-    fakeSharedSettings = {
-      jsonConfigsFrom: sinon.stub().returns({}),
-    };
+    fakeParseJsonConfig = sinon.stub().returns({});
 
     $imports.$mock({
       './config-func-settings-from': fakeConfigFuncSettingsFrom,
       './is-browser-extension': fakeIsBrowserExtension,
-      '../../shared/settings': fakeSharedSettings,
+      '../../boot/parse-json-config': { parseJsonConfig: fakeParseJsonConfig },
     });
   });
 
@@ -211,7 +209,7 @@ describe('annotator.config.settingsFrom', function () {
         beforeEach(
           'add a js-hypothesis-config annotations setting',
           function () {
-            fakeSharedSettings.jsonConfigsFrom.returns({
+            fakeParseJsonConfig.returns({
               annotations: 'annotationsFromJSON',
             });
           }
@@ -310,7 +308,7 @@ describe('annotator.config.settingsFrom', function () {
       'when the host page has a js-hypothesis-config with a query setting',
       function () {
         beforeEach('add a js-hypothesis-config query setting', function () {
-          fakeSharedSettings.jsonConfigsFrom.returns({
+          fakeParseJsonConfig.returns({
             query: 'queryFromJSON',
           });
         });
@@ -468,7 +466,7 @@ describe('annotator.config.settingsFrom', function () {
       },
     ].forEach(function (test) {
       it(test.it, function () {
-        fakeSharedSettings.jsonConfigsFrom.returns({
+        fakeParseJsonConfig.returns({
           showHighlights: test.input,
         });
         const settings = settingsFrom(fakeWindow());
@@ -496,7 +494,7 @@ describe('annotator.config.settingsFrom', function () {
       });
 
       it("doesn't read the setting from the host page, defaults to 'always'", function () {
-        fakeSharedSettings.jsonConfigsFrom.returns({
+        fakeParseJsonConfig.returns({
           showHighlights: 'never',
         });
         fakeConfigFuncSettingsFrom.returns({
@@ -627,7 +625,7 @@ describe('annotator.config.settingsFrom', function () {
         specify(test.specify, function () {
           fakeIsBrowserExtension.returns(test.isBrowserExtension);
           fakeConfigFuncSettingsFrom.returns(test.configFuncSettings);
-          fakeSharedSettings.jsonConfigsFrom.returns(test.jsonSettings);
+          fakeParseJsonConfig.returns(test.jsonSettings);
           const settings = settingsFrom(fakeWindow());
 
           const setting = settings.hostPageSetting('foo', {

src/boot/index.js

@@ -9,14 +9,14 @@
 
 /* global __MANIFEST__ */
 
-import { jsonConfigsFrom } from '../shared/settings';
+import { parseJsonConfig } from './parse-json-config';
 
 import boot from './boot';
 import processUrlTemplate from './url-template';
 import { isBrowserSupported } from './browser-check';
 
 if (isBrowserSupported()) {
-  const settings = jsonConfigsFrom(document);
+  const settings = parseJsonConfig(document);
   boot(document, {
     assetRoot: processUrlTemplate(settings.assetRoot || '__ASSET_ROOT__'),
     // @ts-ignore - `__MANIFEST__` is injected by the build script

src/shared/settings.js → src/boot/parse-json-config.js

@@ -25,7 +25,7 @@ function assign(dest, src) {
  *
  * @param {Document|Element} document - The root element to search.
  */
-export function jsonConfigsFrom(document) {
+export function parseJsonConfig(document) {
   const config = {};
   const settingsElements = document.querySelectorAll(
     'script.js-hypothesis-config'

src/boot/test/parse-json-config-test.js

+import { parseJsonConfig } from '../parse-json-config';
+
+describe('#parseJsonConfig', function () {
+  const sandbox = sinon.createSandbox();
+
+  function appendJSHypothesisConfig(document_, jsonString) {
+    const el = document_.createElement('script');
+    el.type = 'application/json';
+    el.textContent = jsonString;
+    el.classList.add('js-hypothesis-config');
+    el.classList.add('js-settings-test');
+    document_.body.appendChild(el);
+  }
+
+  afterEach(function () {
+    // Remove test config scripts.
+    const elements = document.querySelectorAll('.js-settings-test');
+    for (let i = 0; i < elements.length; i++) {
+      elements[i].remove();
+    }
+
+    sandbox.restore();
+  });
+
+  context('when there are no JSON scripts', function () {
+    it('returns {}', function () {
+      assert.deepEqual(parseJsonConfig(document), {});
+    });
+  });
+
+  context("when there's JSON scripts with no top-level objects", function () {
+    beforeEach('add JSON scripts with no top-level objects', function () {
+      appendJSHypothesisConfig(document, 'null');
+      appendJSHypothesisConfig(document, '23');
+      appendJSHypothesisConfig(document, 'true');
+    });
+
+    it('ignores them', function () {
+      assert.deepEqual(parseJsonConfig(document), {});
+    });
+  });
+
+  context("when there's a JSON script with a top-level array", function () {
+    beforeEach('add a JSON script containing a top-level array', function () {
+      appendJSHypothesisConfig(document, '["a", "b", "c"]');
+    });
+
+    it('returns the array, parsed into an object', function () {
+      assert.deepEqual(parseJsonConfig(document), { 0: 'a', 1: 'b', 2: 'c' });
+    });
+  });
+
+  context("when there's a JSON script with a top-level string", function () {
+    beforeEach('add a JSON script with a top-level string', function () {
+      appendJSHypothesisConfig(document, '"hi"');
+    });
+
+    it('returns the string, parsed into an object', function () {
+      assert.deepEqual(parseJsonConfig(document), { 0: 'h', 1: 'i' });
+    });
+  });
+
+  context("when there's a JSON script containing invalid JSON", function () {
+    beforeEach('stub console.warn()', function () {
+      sandbox.stub(console, 'warn');
+    });
+
+    beforeEach('add a JSON script containing invalid JSON', function () {
+      appendJSHypothesisConfig(document, 'this is not valid json');
+    });
+
+    it('logs a warning', function () {
+      parseJsonConfig(document);
+
+      assert.called(console.warn);
+    });
+
+    it('returns {}', function () {
+      assert.deepEqual(parseJsonConfig(document), {});
+    });
+
+    it('still returns settings from other JSON scripts', function () {
+      appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
+
+      assert.deepEqual(parseJsonConfig(document), { foo: 'FOO', bar: 'BAR' });
+    });
+  });
+
+  context("when there's a JSON script with an empty object", function () {
+    beforeEach('add a JSON script containing an empty object', function () {
+      appendJSHypothesisConfig(document, '{}');
+    });
+
+    it('ignores it', function () {
+      assert.deepEqual(parseJsonConfig(document), {});
+    });
+  });
+
+  context("when there's a JSON script containing some settings", function () {
+    beforeEach('add a JSON script containing some settings', function () {
+      appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
+    });
+
+    it('returns the settings', function () {
+      assert.deepEqual(parseJsonConfig(document), { foo: 'FOO', bar: 'BAR' });
+    });
+  });
+
+  context('when there are JSON scripts with different settings', function () {
+    beforeEach('add some JSON scripts with different settings', function () {
+      appendJSHypothesisConfig(document, '{"foo": "FOO"}');
+      appendJSHypothesisConfig(document, '{"bar": "BAR"}');
+      appendJSHypothesisConfig(document, '{"gar": "GAR"}');
+    });
+
+    it('merges them all into one returned object', function () {
+      assert.deepEqual(parseJsonConfig(document), {
+        foo: 'FOO',
+        bar: 'BAR',
+        gar: 'GAR',
+      });
+    });
+  });
+
+  context('when multiple JSON scripts contain the same setting', function () {
+    beforeEach('add some JSON scripts with different settings', function () {
+      appendJSHypothesisConfig(document, '{"foo": "first"}');
+      appendJSHypothesisConfig(document, '{"foo": "second"}');
+      appendJSHypothesisConfig(document, '{"foo": "third"}');
+    });
+
+    specify(
+      'settings from later in the page override ones from earlier',
+      function () {
+        assert.equal(parseJsonConfig(document).foo, 'third');
+      }
+    );
+  });
+});

src/shared/test/settings-test.js

-import * as settings from '../settings';
-
-const sandbox = sinon.createSandbox();
-
-describe('settings', function () {
-  afterEach('reset the sandbox', function () {
-    sandbox.restore();
-  });
-
-  describe('#jsonConfigsFrom', function () {
-    const jsonConfigsFrom = settings.jsonConfigsFrom;
-
-    function appendJSHypothesisConfig(document_, jsonString) {
-      const el = document_.createElement('script');
-      el.type = 'application/json';
-      el.textContent = jsonString;
-      el.classList.add('js-hypothesis-config');
-      el.classList.add('js-settings-test');
-      document_.body.appendChild(el);
-    }
-
-    afterEach('remove js-hypothesis-config tags', function () {
-      const elements = document.querySelectorAll('.js-settings-test');
-      for (let i = 0; i < elements.length; i++) {
-        elements[i].remove();
-      }
-    });
-
-    context('when there are no JSON scripts', function () {
-      it('returns {}', function () {
-        assert.deepEqual(jsonConfigsFrom(document), {});
-      });
-    });
-
-    context("when there's JSON scripts with no top-level objects", function () {
-      beforeEach('add JSON scripts with no top-level objects', function () {
-        appendJSHypothesisConfig(document, 'null');
-        appendJSHypothesisConfig(document, '23');
-        appendJSHypothesisConfig(document, 'true');
-      });
-
-      it('ignores them', function () {
-        assert.deepEqual(jsonConfigsFrom(document), {});
-      });
-    });
-
-    context("when there's a JSON script with a top-level array", function () {
-      beforeEach('add a JSON script containing a top-level array', function () {
-        appendJSHypothesisConfig(document, '["a", "b", "c"]');
-      });
-
-      it('returns the array, parsed into an object', function () {
-        assert.deepEqual(jsonConfigsFrom(document), { 0: 'a', 1: 'b', 2: 'c' });
-      });
-    });
-
-    context("when there's a JSON script with a top-level string", function () {
-      beforeEach('add a JSON script with a top-level string', function () {
-        appendJSHypothesisConfig(document, '"hi"');
-      });
-
-      it('returns the string, parsed into an object', function () {
-        assert.deepEqual(jsonConfigsFrom(document), { 0: 'h', 1: 'i' });
-      });
-    });
-
-    context("when there's a JSON script containing invalid JSON", function () {
-      beforeEach('stub console.warn()', function () {
-        sandbox.stub(console, 'warn');
-      });
-
-      beforeEach('add a JSON script containing invalid JSON', function () {
-        appendJSHypothesisConfig(document, 'this is not valid json');
-      });
-
-      it('logs a warning', function () {
-        jsonConfigsFrom(document);
-
-        assert.called(console.warn);
-      });
-
-      it('returns {}', function () {
-        assert.deepEqual(jsonConfigsFrom(document), {});
-      });
-
-      it('still returns settings from other JSON scripts', function () {
-        appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
-
-        assert.deepEqual(jsonConfigsFrom(document), { foo: 'FOO', bar: 'BAR' });
-      });
-    });
-
-    context("when there's a JSON script with an empty object", function () {
-      beforeEach('add a JSON script containing an empty object', function () {
-        appendJSHypothesisConfig(document, '{}');
-      });
-
-      it('ignores it', function () {
-        assert.deepEqual(jsonConfigsFrom(document), {});
-      });
-    });
-
-    context("when there's a JSON script containing some settings", function () {
-      beforeEach('add a JSON script containing some settings', function () {
-        appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
-      });
-
-      it('returns the settings', function () {
-        assert.deepEqual(jsonConfigsFrom(document), { foo: 'FOO', bar: 'BAR' });
-      });
-    });
-
-    context('when there are JSON scripts with different settings', function () {
-      beforeEach('add some JSON scripts with different settings', function () {
-        appendJSHypothesisConfig(document, '{"foo": "FOO"}');
-        appendJSHypothesisConfig(document, '{"bar": "BAR"}');
-        appendJSHypothesisConfig(document, '{"gar": "GAR"}');
-      });
-
-      it('merges them all into one returned object', function () {
-        assert.deepEqual(jsonConfigsFrom(document), {
-          foo: 'FOO',
-          bar: 'BAR',
-          gar: 'GAR',
-        });
-      });
-    });
-
-    context('when multiple JSON scripts contain the same setting', function () {
-      beforeEach('add some JSON scripts with different settings', function () {
-        appendJSHypothesisConfig(document, '{"foo": "first"}');
-        appendJSHypothesisConfig(document, '{"foo": "second"}');
-        appendJSHypothesisConfig(document, '{"foo": "third"}');
-      });
-
-      specify(
-        'settings from later in the page override ones from earlier',
-        function () {
-          assert.equal(jsonConfigsFrom(document).foo, 'third');
-        }
-      );
-    });
-  });
-});

src/sidebar/index.js

 /* global process */
 
-import { jsonConfigsFrom } from '../shared/settings';
+import { parseJsonConfig } from '../boot/parse-json-config';
 import * as rendererOptions from '../shared/renderer-options';
 
 import {
@@ -13,7 +13,7 @@ import { fetchConfig } from './util/fetch-config';
 import * as sentry from './util/sentry';
 
 // Read settings rendered into sidebar app HTML by service/extension.
-const appConfig = /** @type {import('../types/config').SidebarConfig} */ (jsonConfigsFrom(
+const appConfig = /** @type {import('../types/config').SidebarConfig} */ (parseJsonConfig(
   document
 ));