Add app context to annotator/config/index.js

- Refactor config index to expose a different method for getting the config called `getConfig`. This now takes a app name (context) as a string which may return a subset of config values appropriate for a given application such as the "sidebar" or "notebook".
- Segregate the config keys into various app contexts and omit keys that are either not used in a given application context or need to be excluded (e.g. `annotations` in notebook)
- Fix missing config appType in annotator/config/index so it can propagate to the sidebar property (this was previously not the case)
- Remove assetRoot from annotator/config/index as it is only needed in boot and not any specific app context

src/annotator/config/index.js

 import settingsFrom from './settings';
 import { toBoolean } from '../../shared/type-coercions';
 
+/**
+ * @typedef {'sidebar'|'notebook'|'annotator'|'all'} AppContext
+ */
+
+/**
+ * List of allowed configuration keys per application context. Keys omitted
+ * in a given context will be removed from the relative configs when calling
+ * getConfig.
+ *
+ * @param {AppContext} [appContext] - The name of the app.
+ */
+function configurationKeys(appContext) {
+  const contexts = {
+    annotator: ['clientUrl', 'showHighlights', 'subFrameIdentifier'],
+    sidebar: [
+      'appType',
+      'annotations',
+      'branding',
+      'enableExperimentalNewNoteButton',
+      'externalContainerSelector',
+      'focus',
+      'group',
+      'onLayoutChange',
+      'openSidebar',
+      'query',
+      'requestConfigFromFrame',
+      'services',
+      'showHighlights',
+      'sidebarAppUrl',
+      'theme',
+      'usernameUrl',
+    ],
+    notebook: [
+      'branding',
+      'group',
+      'notebookAppUrl',
+      'requestConfigFromFrame',
+      'services',
+      'theme',
+      'usernameUrl',
+    ],
+  };
+
+  switch (appContext) {
+    case 'annotator':
+      return contexts.annotator;
+    case 'sidebar':
+      return contexts.sidebar;
+    case 'notebook':
+      return contexts.notebook;
+    case 'all':
+      // Complete list of configuration keys used for testing.
+      return [...contexts.annotator, ...contexts.sidebar, ...contexts.notebook];
+    default:
+      throw new Error(`Invalid application context used: "${appContext}"`);
+  }
+}
+
 /**
  * Reads the Hypothesis configuration from the environment.
  *
+ * @param {string[]} settingsKeys - List of settings that should be returned.
  * @param {Window} window_ - The Window object to read config from.
  */
-export default function configFrom(window_) {
+function configFrom(settingsKeys, window_) {
   const settings = settingsFrom(window_);
-  return {
+  const allConfigSettings = {
     annotations: settings.annotations,
-    // URL where client assets are served from. Used when injecting the client
-    // into child iframes.
-    assetRoot: settings.hostPageSetting('assetRoot', {
+    appType: settings.hostPageSetting('appType', {
       allowInBrowserExt: true,
     }),
     branding: settings.hostPageSetting('branding'),
@@ -50,4 +107,24 @@ export default function configFrom(window_) {
       'externalContainerSelector'
     ),
   };
+
+  // Only return what we asked for
+  const resultConfig = {};
+  settingsKeys.forEach(key => {
+    resultConfig[key] = allConfigSettings[key];
+  });
+  return resultConfig;
+}
+
+/**
+ * Return the configuration for a given application context.
+ *
+ * @param {AppContext} [appContext] - The name of the app.
+ */
+export function getConfig(appContext = 'annotator', window_ = window) {
+  // Filter the config based on the application context as some config values
+  // may be inappropriate or erroneous for some applications.
+  const filteredKeys = configurationKeys(appContext);
+  const config = configFrom(filteredKeys, window_);
+  return config;
 }

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

-import configFrom from '../index';
+import { getConfig } from '../index';
 import { $imports } from '../index';
 
-describe('annotator.config.index', function () {
+describe('annotator/config/index', function () {
   let fakeSettingsFrom;
 
   beforeEach(() => {
@@ -19,7 +19,7 @@ describe('annotator.config.index', function () {
   });
 
   it('gets the configuration settings', function () {
-    configFrom('WINDOW');
+    getConfig('all', 'WINDOW');
 
     assert.calledOnce(fakeSettingsFrom);
     assert.calledWithExactly(fakeSettingsFrom, 'WINDOW');
@@ -27,10 +27,10 @@ describe('annotator.config.index', function () {
 
   ['sidebarAppUrl', 'query', 'annotations', 'group', 'showHighlights'].forEach(
     settingName => {
-      it('returns the ' + settingName + ' setting', () => {
+      it(`returns the ${settingName} setting`, () => {
         fakeSettingsFrom()[settingName] = 'SETTING_VALUE';
 
-        const config = configFrom('WINDOW');
+        const config = getConfig('all', 'WINDOW');
 
         assert.equal(config[settingName], 'SETTING_VALUE');
       });
@@ -46,65 +46,36 @@ describe('annotator.config.index', function () {
 
     it('throws an error', function () {
       assert.throws(function () {
-        configFrom('WINDOW');
+        getConfig('all', 'WINDOW');
       }, "there's no link");
     });
   });
 
-  ['assetRoot', 'subFrameIdentifier'].forEach(function (settingName) {
-    it(
-      'reads ' +
-        settingName +
-        ' from the host page, even when in a browser extension',
-      function () {
-        configFrom('WINDOW');
-        assert.calledWithExactly(
+  ['appType', 'openSidebar', 'subFrameIdentifier'].forEach(function (
+    settingName
+  ) {
+    it(`reads ${settingName} from the host page, even when in a browser extension`, function () {
+      getConfig('all', 'WINDOW');
+      assert.calledWith(
         fakeSettingsFrom().hostPageSetting,
         settingName,
-          { allowInBrowserExt: true }
-        );
-      }
+        sinon.match({ allowInBrowserExt: true })
       );
     });
-
-  it('reads openSidebar from the host page, even when in a browser extension', function () {
-    configFrom('WINDOW');
-    sinon.assert.calledWith(
-      fakeSettingsFrom().hostPageSetting,
-      'openSidebar',
-      sinon.match({
-        allowInBrowserExt: true,
-        coerce: sinon.match.func,
-      })
-    );
   });
 
   ['branding', 'services'].forEach(function (settingName) {
-    it(
-      'reads ' +
-        settingName +
-        ' from the host page only when in an embedded client',
-      function () {
-        configFrom('WINDOW');
-
-        assert.calledWithExactly(
-          fakeSettingsFrom().hostPageSetting,
-          settingName
-        );
-      }
-    );
+    it(`reads ${settingName} from the host page only when in an embedded client`, function () {
+      getConfig('all', 'WINDOW');
+
+      assert.calledWithExactly(fakeSettingsFrom().hostPageSetting, settingName);
+    });
   });
 
-  [
-    'assetRoot',
-    'branding',
-    'openSidebar',
-    'requestConfigFromFrame',
-    'services',
-  ].forEach(function (settingName) {
-    it('returns the ' + settingName + ' value from the host page', function () {
+  ['branding', 'openSidebar', 'requestConfigFromFrame', 'services'].forEach(
+    function (settingName) {
+      it(`returns the ${settingName} value from the host page`, function () {
         const settings = {
-        assetRoot: 'chrome-extension://1234/client/',
           branding: 'BRANDING_SETTING',
           openSidebar: 'OPEN_SIDEBAR_SETTING',
           requestConfigFromFrame: 'https://embedder.com',
@@ -114,9 +85,62 @@ describe('annotator.config.index', function () {
           return settings[settingName];
         };
 
-      const settingValue = configFrom('WINDOW')[settingName];
+        const settingValue = getConfig('all', 'WINDOW')[settingName];
 
         assert.equal(settingValue, settings[settingName]);
       });
+    }
+  );
+  describe('application contexts', () => {
+    [
+      {
+        app: 'annotator',
+        expectedKeys: ['clientUrl', 'showHighlights', 'subFrameIdentifier'],
+      },
+      {
+        app: 'sidebar',
+        expectedKeys: [
+          'appType',
+          'annotations',
+          'branding',
+          'enableExperimentalNewNoteButton',
+          'externalContainerSelector',
+          'focus',
+          'group',
+          'onLayoutChange',
+          'openSidebar',
+          'query',
+          'requestConfigFromFrame',
+          'services',
+          'showHighlights',
+          'sidebarAppUrl',
+          'theme',
+          'usernameUrl',
+        ],
+      },
+      {
+        app: 'notebook',
+        expectedKeys: [
+          'branding',
+          'group',
+          'notebookAppUrl',
+          'requestConfigFromFrame',
+          'services',
+          'theme',
+          'usernameUrl',
+        ],
+      },
+    ].forEach(test => {
+      it(`ignore values not belonging to "${test.app}" context`, () => {
+        const config = getConfig(test.app, 'WINDOW');
+        assert.deepEqual(Object.keys(config), test.expectedKeys);
+      });
+    });
+  });
+
+  it(`throws an error if an invalid context was passed`, () => {
+    assert.throws(() => {
+      getConfig('fake', 'WINDOW');
+    }, 'Invalid application context used: "fake"');
   });
 });

src/annotator/index.js

@@ -17,7 +17,7 @@ import { registerIcons } from '@hypothesis/frontend-shared';
 import iconSet from './icons';
 registerIcons(iconSet);
 
-import configFrom from './config/index';
+import { getConfig } from './config/index';
 import Guest from './guest';
 import Notebook from './notebook';
 import Sidebar from './sidebar';
@@ -33,34 +33,30 @@ const appLinkEl = /** @type {Element} */ (
   )
 );
 
-const config = configFrom(window);
-
 function init() {
+  const annotatorConfig = getConfig('annotator');
   const isPDF = typeof window_.PDFViewerApplication !== 'undefined';
 
-  if (config.subFrameIdentifier) {
+  if (annotatorConfig.subFrameIdentifier) {
     // Other modules use this to detect if this
     // frame context belongs to hypothesis.
     // Needs to be a global property that's set.
     window_.__hypothesis_frame = true;
   }
 
-  // Load the PDF anchoring/metadata integration.
-  config.documentType = isPDF ? 'pdf' : 'html';
-
   const eventBus = new EventBus();
-  const guest = new Guest(document.body, eventBus, config);
-  const sidebar = !config.subFrameIdentifier
-    ? new Sidebar(document.body, eventBus, guest, config)
+  const guest = new Guest(document.body, eventBus, {
+    ...annotatorConfig,
+    // Load the PDF anchoring/metadata integration.
+    // nb. documentType is an internal config property only
+    documentType: isPDF ? 'pdf' : 'html',
+  });
+  const sidebar = !annotatorConfig.subFrameIdentifier
+    ? new Sidebar(document.body, eventBus, guest, getConfig('sidebar'))
     : null;
   // Clear `annotations` value from the notebook's config to prevent direct-linked
   // annotations from filtering the threads.
-  //
-  // TODO: Refactor configFrom() so it can export application specific configs
-  // for different usages such as the notebook.
-  const notebookConfig = { ...config };
-  notebookConfig.annotations = null;
-  const notebook = new Notebook(document.body, eventBus, notebookConfig);
+  const notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
 
   appLinkEl.addEventListener('destroy', () => {
     sidebar?.destroy();