Merge pull request #2244 from hypothesis/typecheck-utils

Add command to typecheck code and enable it for src/sidebar/util/

Makefile

@@ -39,6 +39,7 @@ endif
 .PHONY: lint
 lint: node_modules/.uptodate
 	yarn run lint
+	yarn run typecheck
 
 .PHONY: docs
 docs: python

package.json

@@ -92,6 +92,7 @@
     "terser": "^4.4.0",
     "through2": "^3.0.0",
     "tiny-emitter": "^2.0.2",
+    "typescript": "^3.9.5",
     "unorm": "^1.3.3",
     "vinyl": "^2.2.0",
     "watchify": "^3.7.0",
@@ -127,6 +128,7 @@
     "checkformatting": "prettier --check '**/*.{js,scss}'",
     "format": "prettier --list-different --write '**/*.{js,scss}'",
     "test": "gulp test",
+    "typecheck": "tsc --build src/tsconfig.json",
     "report-coverage": "codecov -f coverage/coverage-final.json",
     "version": "make clean build/manifest.json"
   }

src/sidebar/build-thread.js

@@ -1,10 +1,28 @@
-/** Default state for new threads, before applying filters etc. */
+/**
+ * @typedef {import('../types/api').Annotation} Annotation
+ *
+ * @typedef Thread
+ * @prop {string} id
+ * @prop {Annotation|undefined} annotation
+ * @prop {Thread|undefined} parent
+ * @prop {boolean} visible
+ * @prop {boolean} collapsed
+ * @prop {Thread[]} children
+ * @prop {number} totalChildren
+ * @prop {'dim'|'highlight'|undefined} highlightState
+ */
+
+/**
+ * Default state for new threads, before applying filters etc.
+ *
+ * @type {Thread}
+ */
 const DEFAULT_THREAD_STATE = {
   /**
    * The ID of this thread. This will be the same as the annotation ID for
    * created annotations or the `$tag` property for new annotations.
    */
-  id: undefined,
+  id: '__default__',
   /**
    * The Annotation which is displayed by this thread.
    *
@@ -128,11 +146,14 @@ function threadAnnotations(annotations) {
   });
 
   const root = {
+    id: 'root',
     annotation: undefined,
     children: roots,
     visible: true,
     collapsed: false,
     totalChildren: roots.length,
+    parent: undefined,
+    highlightState: undefined,
   };
 
   return root;
@@ -157,7 +178,7 @@ function mapThread(thread, mapFn) {
  * Return a sorted copy of an array of threads.
  *
  * @param {Array<Thread>} threads - The list of threads to sort
- * @param {(Annotation,Annotation) => boolean} compareFn
+ * @param {(a: Annotation, b: Annotation) => boolean} compareFn
  * @return {Array<Thread>} Sorted list of threads
  */
 function sort(threads, compareFn) {
@@ -219,8 +240,22 @@ function hasVisibleChildren(thread) {
   });
 }
 
+/**
+ * @typedef Options
+ * @prop {string[]} [selected]
+ * @prop {string[]} [forceVisible]
+ * @prop {(a: Annotation) => boolean} [filterFn]
+ * @prop {(t: Thread) => boolean} [threadFilterFn]
+ * @prop {Object} [expanded]
+ * @prop {Object} [highlighted]
+ * @prop {(a: Annotation, b: Annotation) => boolean} [sortCompareFn]
+ * @prop {(a: Annotation, b: Annotation) => boolean} [replySortCompareFn]
+ */
+
 /**
  * Default options for buildThread()
+ *
+ * @type {Partial<Options>}
  */
 const defaultOpts = {
   /** List of currently selected annotation IDs */

src/sidebar/host-config.js

@@ -6,9 +6,44 @@ import {
   toString,
 } from '../shared/type-coercions';
 
+/**
+ * @typedef RequestConfigFromFrameOptions
+ * @prop {number} ancestorLevel
+ * @prop {string} origin
+ */
+
+/**
+ * Configuration for the client provided by the frame embedding it.
+ *
+ * User-facing documentation exists at
+ * https://h.readthedocs.io/projects/client/en/latest/publishers/config/
+ *
+ * @typedef Config
+ * @prop {string} [annotations] - Direct-linked annotation ID
+ * @prop {string} [group] - Direct-linked group ID
+ * @prop {string} [query] - Initial filter query
+ * @prop {string} [appType] - Method used to load the client
+ * @prop {boolean} [openSidebar] - Whether to open the sidebar on the initial load
+ * @prop {boolean} [showHighlights] - Whether to show highlights
+ * @prop {Object[]} [services] -
+ *   Configuration for the annotation services that the client connects to
+ * @prop {Object} [branding] -
+ *   Theme properties (fonts, colors etc.)
+ * @prop {boolean} [enableExperimentalNewNoteButton] -
+ *   Whether to show the "New note" button on the "Page Notes" tab
+ * @prop {RequestConfigFromFrameOptions|string} [requestConfigFromFrame]
+ *   Origin of the ancestor frame to request configuration from
+ * @prop {string} [theme]
+ *   Name of the base theme to use.
+ * @prop {string} [usernameUrl]
+ *   URL template for username links
+ */
+
 /**
  * Return the app configuration specified by the frame embedding the Hypothesis
  * client.
+ *
+ * @return {Config}
  */
 export default function hostPageConfig(window) {
   const configStr = window.location.hash.slice(1);

src/sidebar/util/account-id.js

@@ -19,6 +19,8 @@ export function parseAccountID(user) {
 
 /**
  * Returns the username part of an account ID or an empty string.
+ *
+ * @param {string} user
  */
 export function username(user) {
   const account = parseAccountID(user);
@@ -30,6 +32,9 @@ export function username(user) {
 
 /**
  * Returns true if the authority is of a 3rd party user.
+ *
+ * @param {string} user
+ * @param {string} authDomain
  */
 export function isThirdPartyUser(user, authDomain) {
   const account = parseAccountID(user);

src/sidebar/util/copy-to-clipboard.js

@@ -16,7 +16,7 @@ export function copyText(text) {
 
   try {
     const range = document.createRange();
-    const selection = document.getSelection();
+    const selection = /** @type {Selection} */ (document.getSelection());
 
     selection.removeAllRanges();
     range.selectNodeContents(temp);

src/sidebar/util/date.js

@@ -5,6 +5,8 @@ let formatter;
 /**
  * Returns a standard human-readable representation
  * of a date and time.
+ *
+ * @param {Date} date
  */
 export function format(date) {
   if (typeof Intl !== 'undefined' && Intl.DateTimeFormat) {

src/sidebar/util/disable-opener-for-external-links.js

@@ -17,8 +17,10 @@
  */
 export default function disableOpenerForExternalLinks(root) {
   root.addEventListener('click', event => {
-    if (event.target.tagName === 'A') {
-      const linkEl = event.target;
+    const target = /** @type {HTMLElement} */ (event.target);
+
+    if (target.tagName === 'A') {
+      const linkEl = /** @type {HTMLAnchorElement} */ (target);
       if (linkEl.target === '_blank') {
         linkEl.rel = 'noopener';
       }

src/sidebar/util/dom.js

@@ -27,9 +27,10 @@ export function getElementHeightWithMargins(element) {
  * Attach listeners for one or multiple events to an element and return a
  * function that removes the listeners.
  *
- * @param {Element}
+ * @param {HTMLElement} element
  * @param {string[]} events
- * @param {(event: Event) => any} listener
+ * @param {EventListener} listener
+ * @param {Object} options
  * @param {boolean} [options.useCapture]
  * @return {function} Function which removes the event listeners.
  */

src/sidebar/util/fetch-config.js

@@ -24,7 +24,7 @@ function ancestors(window_) {
  * Returns the global embedder ancestor frame.
  *
  * @param {number} levels - Number of ancestors levels to ascend.
- * @param {Window=} window
+ * @param {Window=} window_
  * @return {Window}
  */
 function getAncestorFrame(levels, window_ = window) {
@@ -57,6 +57,7 @@ function fetchConfigFromAncestorFrame(origin, window_ = window) {
       ancestor,
       origin,
       'requestConfig',
+      [],
       timeout
     );
     configResponses.push(result);
@@ -86,7 +87,7 @@ function fetchConfigLegacy(appConfig, window_ = window) {
   const hostPageConfig = hostConfig(window_);
 
   let embedderConfig;
-  const origin = hostPageConfig.requestConfigFromFrame;
+  const origin = /** @type string */ (hostPageConfig.requestConfigFromFrame);
   embedderConfig = fetchConfigFromAncestorFrame(origin, window_);
 
   return embedderConfig.then(embedderConfig => {
@@ -154,7 +155,7 @@ async function fetchConfigRpc(appConfig, parentFrame, origin) {
  *  already have the `services` value
  * @param {function} rpcCall - RPC method
  *  (method, args, timeout) => Promise
- * @return {Object} - The mutated settings
+ * @return {Promise<Object>} - The mutated settings
  */
 async function fetchGroupsAsync(config, rpcCall) {
   if (Array.isArray(config.services)) {
@@ -170,6 +171,7 @@ async function fetchGroupsAsync(config, rpcCall) {
   }
   return config;
 }
+
 /**
  * Fetch the host configuration and merge it with the app configuration from h.
  *
@@ -180,10 +182,11 @@ async function fetchGroupsAsync(config, rpcCall) {
  *
  * @param {Object} appConfig - Settings rendered into `app.html` by the h service.
  * @param {Window} window_ - Test seam.
- * @return {Object} - The merged settings.
+ * @return {Promise<Object>} - The merged settings.
  */
 export async function fetchConfig(appConfig, window_ = window) {
   const hostPageConfig = hostConfig(window);
+
   const requestConfigFromFrame = hostPageConfig.requestConfigFromFrame;
 
   if (!requestConfigFromFrame) {

src/sidebar/util/group-list-item-common.js

@@ -1,3 +1,11 @@
+/**
+ * @typedef {import('../../types/api').Group} Group
+ */
+
+/**
+ * @param {Group} group
+ * @return {string}
+ */
 export function orgName(group) {
   return group.organization && group.organization.name;
 }

src/sidebar/util/group-organizations.js

@@ -1,5 +1,9 @@
 import immutable from './immutable';
 
+/**
+ * @typedef {import('../../types/api').Group} Group
+ */
+
 // TODO: Update when this is a property available on the API response
 const DEFAULT_ORG_ID = '__default__';
 

src/sidebar/util/groups.js

@@ -1,3 +1,5 @@
+/** @typedef {import('../../types/api').Group} Group */
+
 import escapeStringRegexp from 'escape-string-regexp';
 import serviceConfig from '../service-config';
 

src/sidebar/util/is-third-party-service.js

@@ -9,7 +9,7 @@ import serviceConfig from '../service-config';
  * If no custom annotation services are configured then return `false`.
  *
  * @param {Object} settings - the sidebar settings object
- *
+ * @return {boolean}
  */
 export default function isThirdPartyService(settings) {
   const service = serviceConfig(settings);

src/sidebar/util/memoize.js

@@ -4,6 +4,11 @@
  *
  * The argument to the input function may be of any type and is compared
  * using reference equality.
+ *
+ * @template Arg
+ * @template Result
+ * @param {(arg: Arg) => Result} fn
+ * @return {(arg: Arg) => Result}
  */
 export default function memoize(fn) {
   if (fn.length !== 1) {

src/sidebar/util/oauth-client.js

@@ -192,6 +192,8 @@ export default class OAuthClient {
         response_type: 'code',
         state: state,
       });
+
+    // @ts-ignore - TS doesn't know about `location = <string>`.
     authWindow.location = authUrl;
 
     return authResponse.then(rsp => rsp.code);
@@ -253,10 +255,10 @@ export default class OAuthClient {
       })
       .replace(/&/g, ',');
 
-    return $window.open(
+    return /** @type {Window} */ ($window.open(
       'about:blank',
       'Log in to Hypothesis',
       authWindowSettings
-    );
+    ));
   }
 }

src/sidebar/util/observe-element-size.js

@@ -12,7 +12,9 @@
  * @return {() => void}
  */
 export default function observeElementSize(element, onSizeChanged) {
+  // @ts-ignore - TS is missing `ResizeObserver` type definition
   if (typeof ResizeObserver !== 'undefined') {
+    // @ts-ignore
     const observer = new ResizeObserver(() =>
       onSizeChanged(element.clientWidth, element.clientHeight)
     );

src/sidebar/util/permissions.js

@@ -58,7 +58,7 @@ export function sharedPermissions(userid, groupid) {
  * Return the default permissions for an annotation in a given group.
  *
  * @param {string} userid - User ID of the author
- * @param {string} groupId - ID of the group the annotation is being shared
+ * @param {string} groupid - ID of the group the annotation is being shared
  * with
  * @return {Permissions}
  */

src/sidebar/util/postmessage-json-rpc.js

@@ -21,10 +21,9 @@ function createTimeout(delay, message) {
  * @param {string} origin - Origin filter for `window.postMessage` call
  * @param {string} method - Name of the JSON-RPC method
  * @param {any[]} params - Parameters of the JSON-RPC method
- * @param {number|null} [number=2000] timeout - Maximum time to wait in
- *  ms or null or 0 to ignore timeout.
- * @param [Window] window_ - Test seam.
- * @param [id] id - Test seam.
+ * @param {number} [timeout] - Maximum time to wait in ms
+ * @param {Window} [window_] - Test seam.
+ * @param {string} [id] - Test seam.
  * @return {Promise<any>} - A Promise for the response to the call
  */
 export function call(

src/sidebar/util/random.js

@@ -6,10 +6,11 @@ function byteToHex(val) {
 /**
  * Generate a random hex string of `len` chars.
  *
- * @param {number} - An even-numbered length string to generate.
+ * @param {number} len - An even-numbered length string to generate.
  * @return {string}
  */
 export function generateHexString(len) {
+  // @ts-ignore - TS doesn't know about `msCrypto`.
   const crypto = window.crypto || window.msCrypto; /* IE 11 */
   const bytes = new Uint8Array(len / 2);
   crypto.getRandomValues(bytes);