Commit d333f326 authored by Robert Knight's avatar Robert Knight

Refine types of `config` parameters in annotator code

Various functions and constructors in the annotator code accepted a `config`
parameter with an uninformative `Record<string, any>` type.  Replace these types
with more specific ones that specify required / known optional properties
directly and use `Record<string, unknown>` for properties which are forwarded to
other contexts.

Also improve the documentation for the `getConfig` function which serves
as the entry point for reading configuration in the annotator.
parent ff711a53
......@@ -5,9 +5,18 @@ import classnames from 'classnames';
import { addConfigFragment } from '../../shared/config-fragment';
import { createAppConfig } from '../config/app';
/**
* Configuration used to launch the notebook application.
*
* This includes the URL for the iframe and configuration to pass to the
* application using a config fragment (see {@link addConfigFragment}).
*
* @typedef {{ notebookAppUrl: string } & Record<string, unknown>} NotebookConfig
*/
/**
* @typedef NotebookIframeProps
* @prop {Record<string, any>} config
* @prop {NotebookConfig} config
* @prop {string} groupId
*/
......@@ -40,7 +49,7 @@ function NotebookIframe({ config, groupId }) {
/**
* @typedef NotebookModalProps
* @prop {import('../util/emitter').EventBus} eventBus
* @prop {Record<string, any>} config
* @prop {NotebookConfig} config
*/
/**
......
......@@ -4,7 +4,6 @@ import { toBoolean } from '../../shared/type-coercions';
import { urlFromLinkTag } from './url-from-link-tag';
/**
* @typedef {'sidebar'|'notebook'|'annotator'|'all'} AppContext
* @typedef {import('./settings').SettingsGetters} SettingsGetters
* @typedef {(settings: SettingsGetters, name: string) => any} ValueGetter
*
......@@ -21,13 +20,18 @@ import { urlFromLinkTag } from './url-from-link-tag';
*/
/**
* List of allowed configuration keys per application context. Keys omitted
* in a given context will be removed from the relative configs when calling
* getConfig.
* Named subset of the Hypothesis client configuration that is relevant in
* a particular context.
*
* @param {AppContext} [appContext] - The name of the app.
* @typedef {'sidebar'|'notebook'|'annotator'|'all'} Context
*/
function configurationKeys(appContext) {
/**
* Returns the configuration keys that are relevant to a particular context.
*
* @param {Context} context
*/
function configurationKeys(context) {
const contexts = {
annotator: ['clientUrl', 'contentPartner', 'subFrameIdentifier'],
sidebar: [
......@@ -59,7 +63,7 @@ function configurationKeys(appContext) {
],
};
switch (appContext) {
switch (context) {
case 'annotator':
return contexts.annotator;
case 'sidebar':
......@@ -70,7 +74,7 @@ function configurationKeys(appContext) {
// Complete list of configuration keys used for testing.
return [...contexts.annotator, ...contexts.sidebar, ...contexts.notebook];
default:
throw new Error(`Invalid application context used: "${appContext}"`);
throw new Error(`Invalid application context used: "${context}"`);
}
}
......@@ -81,6 +85,7 @@ function getHostPageSetting(settings, name) {
/**
* Definitions of configuration keys
*
* @type {ConfigDefinitionMap}
*/
const configDefinitions = {
......@@ -192,11 +197,19 @@ const configDefinitions = {
};
/**
* Return the configuration for a given application context.
* Return the subset of Hypothesis client configuration that is relevant in
* a particular context.
*
* See https://h.readthedocs.io/projects/client/en/latest/publishers/config/
* for details of all available configuration and the different ways they
* can be included on the page. In addition to the configuration provided by
* the embedder, the boot script also passes some additional configuration
* to the annotator, such as URLs of the various sub-applications and the
* boot script itself.
*
* @param {AppContext} [appContext] - The name of the app.
* @param {Context} context
*/
export function getConfig(appContext = 'annotator', window_ = window) {
export function getConfig(context, window_ = window) {
const settings = settingsFrom(window_);
/** @type {Record<string, unknown>} */
......@@ -204,9 +217,8 @@ 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.
let filteredKeys = configurationKeys(appContext);
filteredKeys.forEach(name => {
const configDef = configDefinitions[name];
for (let key of configurationKeys(context)) {
const configDef = configDefinitions[key];
const hasDefault = configDef.defaultValue !== undefined; // A default could be null
const isURLFromBrowserExtension = isBrowserExtension(
urlFromLinkTag(window_, 'sidebar', 'html')
......@@ -217,25 +229,25 @@ export function getConfig(appContext = 'annotator', window_ = window) {
// If the value is not allowed here, then set to the default if provided, otherwise ignore
// the key:value pair
if (hasDefault) {
config[name] = configDef.defaultValue;
config[key] = configDef.defaultValue;
}
return;
continue;
}
// Get the value from the configuration source
const value = configDef.getValue(settings, name);
const value = configDef.getValue(settings, key);
if (value === undefined) {
// If there is no value (e.g. undefined), then set to the default if provided,
// otherwise ignore the config key:value pair
if (hasDefault) {
config[name] = configDef.defaultValue;
config[key] = configDef.defaultValue;
}
return;
continue;
}
// Finally, run the value through an optional coerce method
config[name] = configDef.coerce ? configDef.coerce(value) : value;
});
config[key] = configDef.coerce ? configDef.coerce(value) : value;
}
return config;
}
......@@ -93,6 +93,17 @@ function removeTextSelection() {
document.getSelection()?.removeAllRanges();
}
/**
* Subset of the Hypothesis client configuration that is used by {@link Guest}.
*
* @typedef GuestConfig
* @prop {string} [subFrameIdentifier] - An identifier used by this guest to
* identify the current frame when communicating with the sidebar. This is
* only set in non-host frames.
* @prop {'jstor'} [contentPartner] - Configures a banner or other indicators
* showing where the content has come from.
*/
/**
* `Guest` is the central class of the annotator that handles anchoring (locating)
* annotations in the document when they are fetched by the sidebar, rendering
......@@ -121,7 +132,7 @@ export class Guest {
* @param {HTMLElement} element -
* The root element in which the `Guest` instance should be able to anchor
* or create annotations. In an ordinary web page this typically `document.body`.
* @param {Record<string, any>} [config]
* @param {GuestConfig} [config]
* @param {Window} [hostFrame] -
* Host frame which this guest is associated with. This is expected to be
* an ancestor of the guest frame. It may be same or cross origin.
......
......@@ -6,6 +6,15 @@ import { onNextDocumentReady, FrameObserver } from './frame-observer';
* @typedef {import('../types/annotator').Destroyable} Destroyable
*/
/**
* Options for injecting the client into child frames.
*
* This includes the URL of the client's boot script, plus configuration
* for the client when it loads in the child frame.
*
* @typedef {{ clientUrl: string } & Record<string, unknown>} InjectConfig
*/
/**
* HypothesisInjector injects the Hypothesis client into same-origin iframes.
*
......@@ -19,8 +28,7 @@ export class HypothesisInjector {
/**
* @param {Element} element - root of the DOM subtree to watch for the
* addition and removal of annotatable iframes
* @param {Record<string, any>} config - Annotator configuration that is
* injected, along with the Hypothesis client, into the child iframes
* @param {InjectConfig} config
*/
constructor(element, config) {
this._config = config;
......@@ -59,8 +67,7 @@ function hasHypothesis(iframe) {
* See {@link onDocumentReady}.
*
* @param {HTMLIFrameElement} frame
* @param {Record<string, any>} config - Annotator configuration that is
* injected, along with the Hypothesis client, into the child iframes
* @param {InjectConfig} config -
*/
export async function injectClient(frame, config) {
if (hasHypothesis(frame)) {
......
......@@ -34,6 +34,14 @@ const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
)
);
/**
* @typedef {import('./components/NotebookModal').NotebookConfig} NotebookConfig
* @typedef {import('./guest').GuestConfig} GuestConfig
* @typedef {import('./hypothesis-injector').InjectConfig} InjectConfig
* @typedef {import('./sidebar').SidebarConfig} SidebarConfig
* @typedef {import('./sidebar').SidebarContainerConfig} SidebarContainerConfig
*/
/**
* Entry point for the part of the Hypothesis client that runs in the page being
* annotated.
......@@ -47,7 +55,9 @@ const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
* client is initially loaded, is also the only guest frame.
*/
function init() {
const annotatorConfig = getConfig('annotator');
const annotatorConfig = /** @type {GuestConfig & InjectConfig} */ (
getConfig('annotator')
);
const hostFrame = annotatorConfig.subFrameIdentifier ? window.parent : window;
......@@ -59,7 +69,7 @@ function init() {
const removeWorkaround = installPortCloseWorkaroundForSafari();
destroyables.push({ destroy: removeWorkaround });
const sidebarConfig = getConfig('sidebar');
const sidebarConfig = /** @type {SidebarConfig} */ (getConfig('sidebar'));
const hypothesisAppsOrigin = new URL(
/** @type {string} */ (sidebarConfig.sidebarAppUrl)
......@@ -71,7 +81,7 @@ function init() {
const notebook = new Notebook(
document.body,
eventBus,
getConfig('notebook')
/** @type {NotebookConfig} */ (getConfig('notebook'))
);
portProvider.on('frameConnected', (source, port) =>
......
......@@ -11,6 +11,7 @@ import { injectClient } from '../hypothesis-injector';
* @typedef {import('../../types/annotator').Integration} Integration
* @typedef {import('../../types/annotator').Selector} Selector
* @typedef {import('../../types/annotator').SidebarLayout} SidebarLayout
* @typedef {import('../hypothesis-injector').InjectConfig} InjectConfig
*/
// When activating side-by-side mode for VitalSource PDF documents, make sure
......@@ -67,8 +68,8 @@ export function vitalSourceFrameRole(window_ = window) {
*/
export class VitalSourceInjector {
/**
* @param {Record<string, any>} config - Annotator configuration that is
* injected, along with the Hypothesis client, into the book content iframes
* @param {InjectConfig} config - Configuration for injecting the client into
* book content frames
*/
constructor(config) {
const bookElement = findBookElement();
......
......@@ -2,7 +2,10 @@ import { createShadowRoot } from './util/shadow-root';
import { render } from 'preact';
import NotebookModal from './components/NotebookModal';
/** @typedef {import('../types/annotator').Destroyable} Destroyable */
/**
* @typedef {import('../types/annotator').Destroyable} Destroyable
* @typedef {import('./components/NotebookModal').NotebookConfig} NotebookConfig
*/
/** @implements {Destroyable} */
export class Notebook {
......@@ -10,9 +13,9 @@ export class Notebook {
* @param {HTMLElement} element
* @param {import('./util/emitter').EventBus} eventBus -
* Enables communication between components sharing the same eventBus
* @param {Record<string, any>} config
* @param {NotebookConfig} config
*/
constructor(element, eventBus, config = {}) {
constructor(element, eventBus, config) {
/**
* Un-styled shadow host for the notebook content.
* This isolates the notebook from the page's styles.
......
......@@ -18,6 +18,7 @@ import { createShadowRoot } from './util/shadow-root';
* @typedef {import('../types/annotator').AnchorPosition} AnchorPosition
* @typedef {import('../types/annotator').SidebarLayout} SidebarLayout
* @typedef {import('../types/annotator').Destroyable} Destroyable
* @typedef {import('../types/config').Service} Service
* @typedef {import('../types/port-rpc-events').GuestToHostEvent} GuestToHostEvent
* @typedef {import('../types/port-rpc-events').HostToGuestEvent} HostToGuestEvent
* @typedef {import('../types/port-rpc-events').HostToSidebarEvent} HostToSidebarEvent
......@@ -27,10 +28,34 @@ import { createShadowRoot } from './util/shadow-root';
// Minimum width to which the iframeContainer can be resized.
export const MIN_RESIZE = 280;
/**
* Client configuration used to launch the sidebar application.
*
* This includes the URL for the iframe and configuration to pass to the
* application using a config fragment (see {@link addConfigFragment}).
*
* @typedef {{ sidebarAppUrl: string } & Record<string, unknown>} SidebarConfig
*/
/**
* Client configuration used by the sidebar container ({@link Sidebar}).
*
* @typedef SidebarContainerConfig
* @prop {Service[]} [services] - Details of the annotation service the
* client should connect to. This includes callbacks provided by the host
* page to handle certain actions in the sidebar (eg. the Login button).
* @prop {string} [externalContainerSelector] - CSS selector of a container
* element in the host page which the sidebar should be added into, instead
* of creating a new container.
* @prop {(layout: SidebarLayout) => void} [onLayoutChange] - Callback that
* allows the host page to react to the sidebar being opened, closed or
* resized
*/
/**
* Create the iframe that will load the sidebar application.
*
* @param {Record<string, unknown>} config
* @param {SidebarConfig} config
* @return {HTMLIFrameElement}
*/
function createSidebarIframe(config) {
......@@ -63,9 +88,9 @@ export class Sidebar {
* @param {HTMLElement} element
* @param {import('./util/emitter').EventBus} eventBus -
* Enables communication between components sharing the same eventBus
* @param {Record<string, any>} [config]
* @param {SidebarContainerConfig & SidebarConfig} config
*/
constructor(element, eventBus, config = {}) {
constructor(element, eventBus, config) {
this._emitter = eventBus.createEmitter();
/**
......@@ -345,7 +370,7 @@ export class Sidebar {
this.show();
});
/** @type {Array<[SidebarToHostEvent, function]>} */
/** @type {Array<[SidebarToHostEvent, Function|undefined]>} */
const eventHandlers = [
['loginRequested', this.onLoginRequest],
['logoutRequested', this.onLogoutRequest],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment