Add missing types for PortRPC method handlers

Previously the arguments of the callback passed to `PortRPC.on` were
inferred as `any`. Make the callback type generic so that the caller is
forced to specify what types the arguments have.

Internally within PortRPC, use `unknown` rather than `any` for RPC
method arguments where possible.

Note that this change does not ensure that an RPC method is called (via `call`)
using arguments of the same types that the handler expects, but at least it is
now easier to check that the `call` and `on` uses match up and that arguments
are typed within the handler.

src/annotator/guest.js

@@ -367,8 +367,10 @@ export class Guest {
   }
 
   async _connectSidebar() {
-    this._sidebarRPC.on('featureFlagsUpdated', flags =>
-      this.features.update(flags)
+    this._sidebarRPC.on(
+      'featureFlagsUpdated',
+      /** @param {Record<string, boolean>} flags */ flags =>
+        this.features.update(flags)
     );
 
     // Handlers for events sent when user hovers or clicks on an annotation card
@@ -409,9 +411,12 @@ export class Guest {
     );
 
     // Handler for controls on the sidebar
-    this._sidebarRPC.on('setHighlightsVisible', showHighlights => {
-      this.setHighlightsVisible(showHighlights);
-    });
+    this._sidebarRPC.on(
+      'setHighlightsVisible',
+      /** @param {boolean} showHighlights */ showHighlights => {
+        this.setHighlightsVisible(showHighlights);
+      }
+    );
 
     this._sidebarRPC.on(
       'deleteAnnotation',

src/annotator/sidebar.js

@@ -329,8 +329,10 @@ export class Sidebar {
     annotationCounts(document.body, this._sidebarRPC);
     sidebarTrigger(document.body, () => this.open());
 
-    this._sidebarRPC.on('featureFlagsUpdated', flags =>
-      this.features.update(flags)
+    this._sidebarRPC.on(
+      'featureFlagsUpdated',
+      /** @param {Record<string, boolean>} flags */ flags =>
+        this.features.update(flags)
     );
 
     this._sidebarRPC.on('connect', () => {

src/shared/messaging/port-rpc.js

@@ -33,14 +33,14 @@ const PROTOCOL = 'frame-rpc';
  * See https://github.com/substack/frame-rpc#protocol
  *
  * @typedef RequestMessage
- * @prop {any[]} arguments
+ * @prop {unknown[]} arguments
  * @prop {string} method
  * @prop {PROTOCOL} protocol
  * @prop {number} sequence
  * @prop {VERSION} version
  *
  * @typedef ResponseMessage
- * @prop {any[]} arguments
+ * @prop {unknown[]} arguments
  * @prop {PROTOCOL} protocol
  * @prop {number} response
  * @prop {VERSION} version
@@ -55,7 +55,7 @@ const PROTOCOL = 'frame-rpc';
  *
  * @param {MessagePort} port
  * @param {string} method
- * @param {any[]} [args]
+ * @param {unknown[]} [args]
  * @param {number} [sequence] - Sequence number used for replies
  */
 function sendCall(port, method, args = [], sequence = -1) {
@@ -125,6 +125,20 @@ function shouldUseSafariWorkaround(userAgent) {
   return true;
 }
 
+/**
+ * Callback type used for RPC method handlers and result callbacks.
+ *
+ * @typedef {(...args: unknown[]) => void} Callback
+ */
+
+/**
+ * @param {any} value
+ * @return {value is Callback}
+ */
+function isCallback(value) {
+  return typeof value === 'function';
+}
+
 /**
  * PortRPC provides remote procedure calls between frames or workers. It uses
  * the Channel Messaging API [1] as a transport.
@@ -164,12 +178,12 @@ export class PortRPC {
     /** @type {MessagePort|null} */
     this._port = null;
 
-    /** @type {Map<string, (...args: any[]) => void>} */
+    /** @type {Map<string, Callback>} */
     this._methods = new Map();
 
     this._sequence = 1;
 
-    /** @type {Map<number, (...args: any[]) => void>} */
+    /** @type {Map<number, Callback>} */
     this._callbacks = new Map();
 
     this._listeners = new ListenerCollection();
@@ -198,7 +212,7 @@ export class PortRPC {
     /**
      * Method and arguments of pending RPC calls made before a port was connected.
      *
-     * @type {Array<[CallMethod, any[]]>}
+     * @type {Array<[CallMethod, unknown[]]>}
      */
     this._pendingCalls = [];
 
@@ -213,14 +227,15 @@ export class PortRPC {
    *
    * All handlers must be registered before {@link connect} is invoked.
    *
+   * @template {function} Handler
    * @param {OnMethod|'close'|'connect'} method
-   * @param {(...args: any[]) => void} handler
+   * @param {Handler} handler
    */
   on(method, handler) {
     if (this._port) {
       throw new Error('Cannot add a method handler after a port is connected');
     }
-    this._methods.set(method, handler);
+    this._methods.set(method, /** @type {any} */ (handler));
   }
 
   /**
@@ -262,7 +277,7 @@ export class PortRPC {
    * which is invoked with the response in the form of (error, result) arguments.
    *
    * @param {CallMethod} method
-   * @param {any[]} args
+   * @param {unknown[]} args
    */
   call(method, ...args) {
     if (!this._port) {
@@ -275,7 +290,7 @@ export class PortRPC {
 
     const seq = this._sequence++;
     const finalArg = args[args.length - 1];
-    if (typeof finalArg === 'function') {
+    if (isCallback(finalArg)) {
       this._callbacks.set(seq, finalArg);
       args = args.slice(0, -1);
     }
@@ -332,7 +347,7 @@ export class PortRPC {
         return;
       }
 
-      /** @param {any[]} args */
+      /** @param {unknown[]} args */
       const callback = (...args) => {
         /** @type {ResponseMessage} */
         const message = {

src/sidebar/services/frame-sync.js

@@ -8,6 +8,7 @@ import { watch } from '../util/watch';
 
 /**
  * @typedef {import('../../types/annotator').AnnotationData} AnnotationData
+ * @typedef {import('../../types/annotator').DocumentMetadata} DocumentMetadata
  * @typedef {import('../../types/api').Annotation} Annotation
  * @typedef {import('../../types/port-rpc-events').SidebarToHostEvent} SidebarToHostEvent
  * @typedef {import('../../types/port-rpc-events').HostToSidebarEvent} HostToSidebarEvent
@@ -16,6 +17,13 @@ import { watch } from '../util/watch';
  * @typedef {import('../store/modules/frames').Frame} Frame
  */
 
+/**
+ * @typedef DocumentInfo
+ * @prop {string|null} frameIdentifier
+ * @prop {string} uri
+ * @prop {DocumentMetadata} metadata
+ */
+
 /**
  * Return a minimal representation of an annotation that can be sent from the
  * sidebar app to a guest frame.
@@ -256,18 +264,22 @@ export class FrameSyncService {
     // guest will make this call once after it connects. To handle updates
     // to the document, we'll need to change `connectFrame` to update rather than
     // add to the frame list.
-    guestRPC.on('documentInfoChanged', info => {
-      this._guestRPC.delete(frameIdentifier);
-
-      frameIdentifier = info.frameIdentifier;
-      this._guestRPC.set(frameIdentifier, guestRPC);
-
-      this._store.connectFrame({
-        id: info.frameIdentifier,
-        metadata: info.metadata,
-        uri: info.uri,
-      });
-    });
+    guestRPC.on(
+      'documentInfoChanged',
+      /** @param {DocumentInfo} info */
+      info => {
+        this._guestRPC.delete(frameIdentifier);
+
+        frameIdentifier = info.frameIdentifier;
+        this._guestRPC.set(frameIdentifier, guestRPC);
+
+        this._store.connectFrame({
+          id: info.frameIdentifier,
+          metadata: info.metadata,
+          uri: info.uri,
+        });
+      }
+    );
 
     // TODO - Close connection if we don't receive a "connect" message within
     // a certain time frame.
@@ -340,18 +352,27 @@ export class FrameSyncService {
       }
     );
 
-    guestRPC.on('showAnnotations', tags => {
-      this._store.selectAnnotations(this._store.findIDsForTags(tags));
-      this._store.selectTab('annotation');
-    });
+    guestRPC.on(
+      'showAnnotations',
+      /** @param {string[]} tags */ tags => {
+        this._store.selectAnnotations(this._store.findIDsForTags(tags));
+        this._store.selectTab('annotation');
+      }
+    );
 
-    guestRPC.on('focusAnnotations', tags => {
-      this._store.focusAnnotations(tags || []);
-    });
+    guestRPC.on(
+      'focusAnnotations',
+      /** @param {string[]} tags */ tags => {
+        this._store.focusAnnotations(tags || []);
+      }
+    );
 
-    guestRPC.on('toggleAnnotationSelection', tags => {
-      this._store.toggleSelectedAnnotations(this._store.findIDsForTags(tags));
-    });
+    guestRPC.on(
+      'toggleAnnotationSelection',
+      /** @param {string[]} tags */ tags => {
+        this._store.toggleSelectedAnnotations(this._store.findIDsForTags(tags));
+      }
+    );
 
     guestRPC.on('openSidebar', () => {
       this._hostRPC.call('openSidebar');
@@ -378,10 +399,15 @@ export class FrameSyncService {
 
     // When user toggles the highlight visibility control in the sidebar container,
     // update the visibility in all the guest frames.
-    this._hostRPC.on('setHighlightsVisible', visible => {
-      this._highlightsVisible = visible;
-      this._guestRPC.forEach(rpc => rpc.call('setHighlightsVisible', visible));
-    });
+    this._hostRPC.on(
+      'setHighlightsVisible',
+      /** @param {boolean} visible */ visible => {
+        this._highlightsVisible = visible;
+        this._guestRPC.forEach(rpc =>
+          rpc.call('setHighlightsVisible', visible)
+        );
+      }
+    );
   }
 
   /**