Commit e847bc40 authored by Robert Knight's avatar Robert Knight

Make sidebar/config/ typecheck with `noImplicitAny`

Add and improve types in src/sidebar/config/ so that this directory
typechecks with `noImplicitAny` enabled.

 - Add "$rpc:requestGroups" as a possible value for the `groups` config
   in types/config.js, which code in sidebar/config/ needs to replace
   with a promise for a list of groups.

   This required a change in `sidebar/services/groups.js` to either
   check for or explicitly assert that `service.groups` is an array at
   this point (and not the string "$rpc:requestGroups").

 - Replace various `object` types with `ConfigFromHost`,
   `ConfigFromSidebar` or `SidebarSettings` as appropriate

 - Rewrite the code at the bottom of `hostPageConfig` that copies
   configuration from the `config=` URL fragment into a sanitized
   `ConfigFromHost` object, so that it is easier to read and add types
   to.
parent 1284e2f2
......@@ -22,7 +22,7 @@ export function addConfigFragment(baseURL, config) {
* Parse configuration from a URL generated by {@link addConfigFragment}.
*
* @param {string} url
* @return {object}
* @return {Record<string, unknown>}
*/
export function parseConfigFragment(url) {
const configStr = new URL(url).hash.slice(1);
......
......@@ -3,6 +3,7 @@ import { hostPageConfig } from './host-config';
import * as postMessageJsonRpc from '../util/postmessage-json-rpc';
/**
* @typedef {import('../../types/config').ConfigFromHost} ConfigFromHost
* @typedef {import('../../types/config').ConfigFromSidebar} ConfigFromSidebar
* @typedef {import('../../types/config').SidebarSettings} SidebarSettings
*/
......@@ -30,8 +31,8 @@ function getAncestorFrame(levels, window_ = window) {
/**
* Merge client configuration from h service with config from the hash fragment.
*
* @param {object} appConfig - App config settings rendered into `app.html` by the h service.
* @param {object} hostPageConfig - App configuration specified by the embedding frame.
* @param {ConfigFromSidebar} appConfig - App config settings rendered into `app.html` by the h service.
* @param {ConfigFromHost} hostPageConfig - App configuration specified by the embedding frame.
* @return {SidebarSettings} - The merged settings.
*/
function fetchConfigEmbed(appConfig, hostPageConfig) {
......@@ -50,10 +51,10 @@ function fetchConfigEmbed(appConfig, hostPageConfig) {
* Use this method to retrieve the config asynchronously from a parent
* frame via RPC. See tests for more details.
*
* @param {object} appConfig - Settings rendered into `app.html` by the h service.
* @param {ConfigFromSidebar} appConfig - Settings rendered into `app.html` by the h service.
* @param {Window} parentFrame - Frame to send call to.
* @param {string} origin - Origin filter for `window.postMessage` call.
* @return {Promise<object>} - The merged settings.
* @return {Promise<SidebarSettings>} - The merged settings.
*/
async function fetchConfigRpc(appConfig, parentFrame, origin) {
const remoteConfig = await postMessageJsonRpc.call(
......@@ -63,7 +64,10 @@ async function fetchConfigRpc(appConfig, parentFrame, origin) {
[],
3000
);
// Closure for the RPC call to scope parentFrame and origin variables.
/**
* @param {string} method
* @param {any[]} args
*/
const rpcCall = (method, args = [], timeout = 3000) =>
postMessageJsonRpc.call(parentFrame, origin, method, args, timeout);
const sidebarSettings = fetchConfigEmbed(appConfig, remoteConfig);
......@@ -81,11 +85,11 @@ async function fetchConfigRpc(appConfig, parentFrame, origin) {
* fill in the `groups` value(s) later when its ready. This helps speed
* up the loading process.
*
* @param {object} config - The configuration object to mutate. This should
* @param {SidebarSettings} config - The configuration object to mutate. This should
* already have the `services` value
* @param {function} rpcCall - RPC method
* (method, args, timeout) => Promise
* @return {Promise<object>} - The mutated settings
* @return {Promise<SidebarSettings>} - The mutated settings
*/
async function fetchGroupsAsync(config, rpcCall) {
if (Array.isArray(config.services)) {
......
/** @typedef {import('../../types/config').SidebarSettings} SidebarSettings */
import { serviceConfig } from './service-config';
/**
* Function that returns apiUrl from the settings object.
*
* @param {object} settings - The settings object
* @param {SidebarSettings} settings - The settings object
* @returns {string} The apiUrl from the service or the default apiUrl from the settings
* @throws {Error} If the settings has a service but the service doesn't have an apiUrl
*
......
......@@ -78,14 +78,21 @@ export function hostPageConfig(window) {
// It is assumed we should expand this list and coerce and eventually
// even validate all such config values.
// See https://github.com/hypothesis/client/issues/1968
/** @type {Record<string, (value: unknown) => unknown>} */
const coercions = {
openSidebar: toBoolean,
/** @param {unknown} value */
requestConfigFromFrame: value => {
if (typeof value === 'string') {
// Legacy `requestConfigFromFrame` value which holds only the origin.
return value;
}
const objectVal = toObject(value);
const objectVal =
/** @type {{ origin: unknown, ancestorLevel: unknown }} */ (
toObject(value)
);
return {
origin: toString(objectVal.origin),
ancestorLevel: toInteger(objectVal.ancestorLevel),
......@@ -93,20 +100,25 @@ export function hostPageConfig(window) {
},
};
return Object.keys(config).reduce((result, key) => {
if (paramWhiteList.indexOf(key) !== -1) {
// Ignore `null` values as these indicate a default value.
// In this case the config value set in the sidebar app HTML config is
// used.
if (config[key] !== null) {
if (coercions[key]) {
// If a coercion method exists, pass it through
result[key] = coercions[key](config[key]);
} else {
result[key] = config[key];
}
}
/** @type {Record<string, unknown>} */
const result = {};
for (let [key, value] of Object.entries(config)) {
if (!paramWhiteList.includes(key)) {
continue;
}
// Ignore `null` values as these indicate a default value.
// In this case the config value set in the sidebar app HTML config is
// used.
if (value === null) {
continue;
}
if (coercions[key]) {
result[key] = coercions[key](value);
} else {
result[key] = value;
}
return result;
}, {});
}
return result;
}
......@@ -36,6 +36,14 @@ function injectOrganizations(groups) {
});
}
/**
* @param {any} value
* @return {value is Promise<unknown>}
*/
function isPromise(value) {
return typeof value?.then === 'function';
}
// `expand` parameter for various groups API calls.
const expandParam = ['organization', 'scopes'];
......@@ -417,10 +425,14 @@ export class GroupsService {
* @return {Promise<Group[]>}
*/
async load() {
if (this._serviceConfig?.groups) {
// The `groups` property may be a list of group IDs or a promise for one,
// if we're in the LMS app and the group list is being fetched asynchronously.
const groupIdsOrPromise = this._serviceConfig?.groups;
if (Array.isArray(groupIdsOrPromise) || isPromise(groupIdsOrPromise)) {
let groupIds = [];
try {
groupIds = await this._serviceConfig.groups;
groupIds = await groupIdsOrPromise;
} catch (e) {
this._toastMessenger.error(
`Unable to fetch group configuration: ${e.message}`
......
......@@ -27,6 +27,7 @@
"annotator/**/*.js",
"boot/**/*.js",
"shared/**/*.js",
"sidebar/config/*.js",
"sidebar/util/*.js",
"types/*.d.ts"
],
......
......@@ -12,7 +12,7 @@
* @prop {string} authority
* @prop {string} grantToken
* @prop {string} [icon]
* @prop {string[]|Promise<string[]>} [groups] -
* @prop {string[]|Promise<string[]>|'$rpc:requestGroups'} [groups] -
* List of groups to show. The embedder can specify an array. In the sidebar
* this may be converted to a Promise if this information is fetched asynchronously.
* @prop {boolean} [allowFlagging]
......
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