Strengthen the types for Bridge events

I have created type definitions for all the event names that are sent across the different frames using various `Bridge`s. It is based on the previous `bridge-events.js`. I broke down the events in four sections based on the direction of the messages: * guest -> sidebar events * host -> sidebar events * sidebar -> guest/s events * sidebar -> host events For those events that didn't have a description I added one. This is more stringent and less verbose than the previous lookup system.
hypothesis · Oct 15, 2021 · e6b39e5 · e6b39e5
1 parent 910b3da
commit e6b39e5
Show file tree

Hide file tree

Showing 22 changed files with 276 additions and 166 deletions.
diff --git a/package.json b/package.json
@@ -123,8 +123,8 @@
   "scripts": {
     "build": "cross-env NODE_ENV=production gulp build",
     "lint": "eslint .",
-    "checkformatting": "prettier --check '**/*.{js,scss}'",
-    "format": "prettier --list-different --write '**/*.{js,scss}'",
+    "checkformatting": "prettier --check '**/*.{js,scss,d.ts}'",
+    "format": "prettier --list-different --write '**/*.{js,scss,d.ts}'",
     "test": "gulp test",
     "typecheck": "tsc --build tsconfig.json",
     "report-coverage": "codecov -f coverage/coverage-final.json",

diff --git a/src/annotator/annotation-counts.js b/src/annotator/annotation-counts.js
@@ -1,5 +1,3 @@
-import events from '../shared/bridge-events';
-
 const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';
 
 /**
@@ -11,8 +9,9 @@ const ANNOTATION_COUNT_ATTR = 'data-hypothesis-annotation-count';
  *   display annotation count.
  * @param {import('../shared/bridge').Bridge} bridge - Channel for host-sidebar communication
  */
+
 export function annotationCounts(rootEl, bridge) {
-  bridge.on(events.PUBLIC_ANNOTATION_COUNT_CHANGED, updateAnnotationCountElems);
+  bridge.on('publicAnnotationCountChanged', updateAnnotationCountElems);
 
   function updateAnnotationCountElems(newCount) {
     const elems = rootEl.querySelectorAll('[' + ANNOTATION_COUNT_ATTR + ']');

diff --git a/src/annotator/annotation-sync.js b/src/annotator/annotation-sync.js
@@ -1,10 +1,17 @@
 /**
- * @typedef {import('../shared/bridge').Bridge} Bridge
  * @typedef {import('../types/annotator').AnnotationData} AnnotationData
  * @typedef {import('../types/annotator').Destroyable} Destroyable
+ * @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
+ * @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
  * @typedef {import('./util/emitter').EventBus} EventBus
  */
 
+/**
+ * @template {GuestToSidebarEvent} T
+ * @template {SidebarToGuestEvent} U
+ * @typedef {import('../shared/bridge').Bridge<T,U>} Bridge
+ */
+
 /**
  * Message sent between the sidebar and annotator about an annotation that has
  * changed.
@@ -26,11 +33,11 @@
 export class AnnotationSync {
   /**
    * @param {EventBus} eventBus - Event bus for communicating with the annotator code (eg. the Guest)
-   * @param {Bridge} bridge - Channel for communicating with the sidebar
+   * @param {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>} bridge - Channel for communicating with the sidebar
    */
   constructor(eventBus, bridge) {
     this._emitter = eventBus.createEmitter();
-    this.bridge = bridge;
+    this._sidebar = bridge;
 
     /**
      * Mapping from annotation tags to annotation objects for annotations which
@@ -43,7 +50,7 @@ export class AnnotationSync {
     this.destroyed = false;
 
     // Relay events from the sidebar to the rest of the annotator.
-    this.bridge.on('deleteAnnotation', (body, callback) => {
+    this._sidebar.on('deleteAnnotation', (body, callback) => {
       if (this.destroyed) {
         callback(null);
         return;
@@ -55,7 +62,7 @@ export class AnnotationSync {
       callback(null);
     });
 
-    this.bridge.on('loadAnnotations', (bodies, callback) => {
+    this._sidebar.on('loadAnnotations', (bodies, callback) => {
       if (this.destroyed) {
         callback(null);
         return;
@@ -70,7 +77,7 @@ export class AnnotationSync {
       if (annotation.$tag) {
         return;
       }
-      this.bridge.call('beforeCreateAnnotation', this._format(annotation));
+      this._sidebar.call('beforeCreateAnnotation', this._format(annotation));
     });
   }
 
@@ -87,7 +94,7 @@ export class AnnotationSync {
       return;
     }
 
-    this.bridge.call(
+    this._sidebar.call(
       'sync',
       annotations.map(ann => this._format(ann))
     );

diff --git a/src/annotator/features.js b/src/annotator/features.js
@@ -1,4 +1,3 @@
-import events from '../shared/bridge-events';
 import warnOnce from '../shared/warn-once';
 
 let _features = {};
@@ -12,7 +11,7 @@ export const features = {
    * @param {import('../shared/bridge').Bridge} bridge - Channel for host-sidebar communication
    */
   init: function (bridge) {
-    bridge.on(events.FEATURE_FLAGS_UPDATED, _set);
+    bridge.on('featureFlagsUpdated', _set);
   },
 
   reset: function () {

diff --git a/src/annotator/guest.js b/src/annotator/guest.js
@@ -25,6 +25,8 @@ import { normalizeURI } from './util/url';
  * @typedef {import('../types/annotator').Destroyable} Destroyable
  * @typedef {import('../types/annotator').SidebarLayout} SidebarLayout
  * @typedef {import('../types/api').Target} Target
+ * @typedef {import('../types/bridge-events').GuestToSidebarEvent} GuestToSidebarEvent
+ * @typedef {import('../types/bridge-events').SidebarToGuestEvent} SidebarToGuestEvent
  * @typedef {import('./util/emitter').EventBus} EventBus
  */
 
@@ -163,7 +165,11 @@ export default class Guest {
     // The "top" guest instance will have this as null since it's in a top frame not a sub frame
     this._frameIdentifier = config.subFrameIdentifier || null;
 
-    // Set up listeners for messages coming from the sidebar to this guest.
+    /**
+     * Channel for sidebar-guest communication.
+     *
+     * @type {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>}
+     */
     this._bridge = new Bridge();
     this._bridge.onConnect(() => {
       this._emitter.publish('panelReady');

diff --git a/src/annotator/sidebar.js b/src/annotator/sidebar.js
@@ -1,7 +1,6 @@
 import Hammer from 'hammerjs';
 
 import { Bridge } from '../shared/bridge';
-import events from '../shared/bridge-events';
 import { ListenerCollection } from '../shared/listener-collection';
 
 import { annotationCounts } from './annotation-counts';
@@ -14,6 +13,8 @@ import { createShadowRoot } from './util/shadow-root';
 
 /**
  * @typedef {import('./guest').default} Guest
+ * @typedef {import('../types/bridge-events').HostToSidebarEvent} HostToSidebarEvent
+ * @typedef {import('../types/bridge-events').SidebarToHostEvent} SidebarToHostEvent
  * @typedef {import('../types/annotator').SidebarLayout} SidebarLayout
  * @typedef {import('../types/annotator').Destroyable} Destroyable
  */
@@ -64,6 +65,11 @@ export default class Sidebar {
   constructor(element, eventBus, guest, config = {}) {
     this._emitter = eventBus.createEmitter();
 
+    /**
+     * Channel for sidebar-host communication.
+     *
+     * @type {Bridge<HostToSidebarEvent,SidebarToHostEvent>}
+     */
     this._sidebarRPC = new Bridge();
 
     /**
@@ -252,12 +258,13 @@ export default class Sidebar {
       this.show();
     });
 
+    /** @type {Array<[import('../types/bridge-events').SidebarToHostEvent, function]>} */
     const eventHandlers = [
-      [events.LOGIN_REQUESTED, this.onLoginRequest],
-      [events.LOGOUT_REQUESTED, this.onLogoutRequest],
-      [events.SIGNUP_REQUESTED, this.onSignupRequest],
-      [events.PROFILE_REQUESTED, this.onProfileRequest],
-      [events.HELP_REQUESTED, this.onHelpRequest],
+      ['loginRequested', this.onLoginRequest],
+      ['logoutRequested', this.onLogoutRequest],
+      ['signupRequested', this.onSignupRequest],
+      ['profileRequested', this.onProfileRequest],
+      ['helpRequested', this.onHelpRequest],
     ];
     eventHandlers.forEach(([event, handler]) => {
       if (handler) {

diff --git a/src/annotator/test/annotation-sync-test.js b/src/annotator/test/annotation-sync-test.js
@@ -1,6 +1,5 @@
-import { EventBus } from '../util/emitter';
-
 import { AnnotationSync } from '../annotation-sync';
+import { EventBus } from '../util/emitter';
 
 describe('AnnotationSync', () => {
   let createAnnotationSync;
@@ -49,7 +48,7 @@ describe('AnnotationSync', () => {
         assert.calledWith(eventStub, ann);
       });
 
-      it("calls the 'deleteAnnotation' event's callback function", done => {
+      it('calls the "deleteAnnotation" event\'s callback function', done => {
         const ann = { id: 1, $tag: 'tag1' };
         const callback = function (err, result) {
           assert.isNull(err);

diff --git a/src/annotator/test/features-test.js b/src/annotator/test/features-test.js
@@ -1,4 +1,3 @@
-import events from '../../shared/bridge-events';
 import { features, $imports } from '../features';
 
 describe('features - annotation layer', () => {
@@ -22,7 +21,7 @@ describe('features - annotation layer', () => {
 
     features.init({
       on: function (topic, handler) {
-        if (topic === events.FEATURE_FLAGS_UPDATED) {
+        if (topic === 'featureFlagsUpdated') {
           featureFlagsUpdateHandler = handler;
         }
       },

diff --git a/src/annotator/test/guest-test.js b/src/annotator/test/guest-test.js
@@ -1,6 +1,5 @@
-import Guest from '../guest';
+import Guest, { $imports } from '../guest';
 import { EventBus } from '../util/emitter';
-import { $imports } from '../guest';
 
 class FakeAdder {
   constructor(container, options) {
@@ -234,7 +233,7 @@ describe('Guest', () => {
   });
 
   describe('events from sidebar', () => {
-    const emitGuestEvent = (event, ...args) => {
+    const emitSidebarEvent = (event, ...args) => {
       for (let [evt, fn] of fakeBridge.on.args) {
         if (event === evt) {
           fn(...args);
@@ -252,7 +251,7 @@ describe('Guest', () => {
           { annotation: { $tag: 'tag2' }, highlights: [highlight1] },
         ];
 
-        emitGuestEvent('focusAnnotations', ['tag1']);
+        emitSidebarEvent('focusAnnotations', ['tag1']);
 
         assert.calledWith(
           highlighter.setHighlightsFocused,
@@ -270,7 +269,7 @@ describe('Guest', () => {
           { annotation: { $tag: 'tag2' }, highlights: [highlight1] },
         ];
 
-        emitGuestEvent('focusAnnotations', ['tag1']);
+        emitSidebarEvent('focusAnnotations', ['tag1']);
 
         assert.calledWith(
           highlighter.setHighlightsFocused,
@@ -282,8 +281,8 @@ describe('Guest', () => {
       it('updates focused tag set', () => {
         const guest = createGuest();
 
-        emitGuestEvent('focusAnnotations', ['tag1']);
-        emitGuestEvent('focusAnnotations', ['tag2', 'tag3']);
+        emitSidebarEvent('focusAnnotations', ['tag1']);
+        emitSidebarEvent('focusAnnotations', ['tag2', 'tag3']);
 
         assert.deepEqual([...guest.focusedAnnotationTags], ['tag2', 'tag3']);
       });
@@ -302,7 +301,7 @@ describe('Guest', () => {
           },
         ];
 
-        emitGuestEvent('scrollToAnnotation', 'tag1');
+        emitSidebarEvent('scrollToAnnotation', 'tag1');
 
         assert.called(fakeIntegration.scrollToAnchor);
         assert.calledWith(fakeIntegration.scrollToAnchor, guest.anchors[0]);
@@ -326,7 +325,7 @@ describe('Guest', () => {
             resolve();
           });
 
-          emitGuestEvent('scrollToAnnotation', 'tag1');
+          emitSidebarEvent('scrollToAnnotation', 'tag1');
         });
       });
 
@@ -345,7 +344,7 @@ describe('Guest', () => {
           event.preventDefault()
         );
 
-        emitGuestEvent('scrollToAnnotation', 'tag1');
+        emitSidebarEvent('scrollToAnnotation', 'tag1');
 
         assert.notCalled(fakeIntegration.scrollToAnchor);
       });
@@ -354,7 +353,7 @@ describe('Guest', () => {
         const guest = createGuest();
 
         guest.anchors = [{ annotation: { $tag: 'tag1' } }];
-        emitGuestEvent('scrollToAnnotation', 'tag1');
+        emitSidebarEvent('scrollToAnnotation', 'tag1');
 
         assert.notCalled(fakeIntegration.scrollToAnchor);
       });
@@ -374,7 +373,7 @@ describe('Guest', () => {
         const eventEmitted = sandbox.stub();
         guest.element.addEventListener('scrolltorange', eventEmitted);
 
-        emitGuestEvent('scrollToAnnotation', 'tag1');
+        emitSidebarEvent('scrollToAnnotation', 'tag1');
 
         assert.notCalled(eventEmitted);
         assert.notCalled(fakeIntegration.scrollToAnchor);
@@ -408,7 +407,7 @@ describe('Guest', () => {
 
         fakeIntegration.getMetadata.resolves(metadata);
 
-        emitGuestEvent(
+        emitSidebarEvent(
           'getDocumentInfo',
           createCallback('https://example.com/test.pdf', metadata, done)
         );
@@ -419,14 +418,14 @@ describe('Guest', () => {
       it('sets visibility of highlights in document', () => {
         const guest = createGuest();
 
-        emitGuestEvent('setVisibleHighlights', true);
+        emitSidebarEvent('setVisibleHighlights', true);
         assert.calledWith(
           highlighter.setHighlightsVisible,
           guest.element,
           true
         );
 
-        emitGuestEvent('setVisibleHighlights', false);
+        emitSidebarEvent('setVisibleHighlights', false);
         assert.calledWith(
           highlighter.setHighlightsVisible,
           guest.element,