Fixed views not reopening on restart by fixing PlatformEventEmitter b…

…ug (#947)

lib/papi-dts/papi.d.ts

@@ -3042,6 +3042,10 @@ declare module 'shared/models/network-object-status.service-model' {
     /**
      * Get a promise that resolves when a network object is registered or rejects if a timeout is hit
      *
+     * @param objectDetailsToMatch Subset of object details on the network object to wait for.
+     *   Compared to object details using {@link isSubset}
+     * @param timeoutInMS Max duration to wait for the network object. If not provided, it will wait
+     *   indefinitely
      * @returns Promise that either resolves to the {@link NetworkObjectDetails} for a network object
      *   once the network object is registered, or rejects if a timeout is provided and the timeout is
      *   reached before the network object is registered

lib/platform-bible-utils/dist/index.d.ts

@@ -14,7 +14,7 @@ export declare class AsyncVariable<T> {
 	 * @param variableName Name to use when logging about this variable
 	 * @param rejectIfNotSettledWithinMS Milliseconds to wait before verifying if the promise was
 	 *   settled (resolved or rejected); will reject if it has not settled by that time. Use -1 if you
-	 *   do not want a timeout at all.
+	 *   do not want a timeout at all. Defaults to 10000 ms
 	 */
 	constructor(variableName: string, rejectIfNotSettledWithinMS?: number);
 	/**
@@ -35,15 +35,15 @@ export declare class AsyncVariable<T> {
 	 *
 	 * @param value This variable's promise will resolve to this value
 	 * @param throwIfAlreadySettled Determines whether to throw if the variable was already resolved
-	 *   or rejected
+	 *   or rejected. Defaults to `false`
 	 */
 	resolveToValue(value: T, throwIfAlreadySettled?: boolean): void;
 	/**
 	 * Reject this variable's promise for the value with the given reason
 	 *
 	 * @param reason This variable's promise will be rejected with this reason
 	 * @param throwIfAlreadySettled Determines whether to throw if the variable was already resolved
-	 *   or rejected
+	 *   or rejected. Defaults to `false`
 	 */
 	rejectWithReason(reason: string, throwIfAlreadySettled?: boolean): void;
 	/** Prevent any further updates to this variable */

lib/platform-bible-utils/src/async-variable.ts

@@ -11,7 +11,7 @@ export default class AsyncVariable<T> {
    * @param variableName Name to use when logging about this variable
    * @param rejectIfNotSettledWithinMS Milliseconds to wait before verifying if the promise was
    *   settled (resolved or rejected); will reject if it has not settled by that time. Use -1 if you
-   *   do not want a timeout at all.
+   *   do not want a timeout at all. Defaults to 10000 ms
    */
   constructor(variableName: string, rejectIfNotSettledWithinMS: number = 10000) {
     this.variableName = variableName;
@@ -54,7 +54,7 @@ export default class AsyncVariable<T> {
    *
    * @param value This variable's promise will resolve to this value
    * @param throwIfAlreadySettled Determines whether to throw if the variable was already resolved
-   *   or rejected
+   *   or rejected. Defaults to `false`
    */
   resolveToValue(value: T, throwIfAlreadySettled: boolean = false): void {
     if (this.resolver) {
@@ -72,7 +72,7 @@ export default class AsyncVariable<T> {
    *
    * @param reason This variable's promise will be rejected with this reason
    * @param throwIfAlreadySettled Determines whether to throw if the variable was already resolved
-   *   or rejected
+   *   or rejected. Defaults to `false`
    */
   rejectWithReason(reason: string, throwIfAlreadySettled: boolean = false): void {
     if (this.rejecter) {

lib/platform-bible-utils/src/platform-event-emitter.model.test.ts

@@ -0,0 +1,56 @@
+import PlatformEventEmitter from './platform-event-emitter.model';
+
+describe('unsubscribing', () => {
+  it('does not prevent other subscribers from running when unsubscribing in a callback', () => {
+    const shouldUnsubscribeSubscriptions = [false, true, false, true, true, false];
+    // Map of event number to which event subscription indices ran for that event number
+    // Purposely making this an array, not a Set, to make sure we catch duplicate runs
+    const subscriptionResults: { [eventNum: string]: number[] } = {};
+
+    const numEventsToEmit = 3;
+    // Array of each event number that should have been run: [0, 1, 2, ..., numEventsToEdit - 1]
+    const eventNumArray = [...Array(numEventsToEmit).keys()];
+    // Array of each event number that should have been run after unsubscribing
+    // (basically eventNumArray without the first event)
+    const [, ...eventNumArrayAfterUnsubscribing] = eventNumArray;
+    let nextEventNum = 0;
+    const emitter = new PlatformEventEmitter<number>();
+    const emitEvent = () => {
+      emitter.emit(nextEventNum);
+      nextEventNum += 1;
+    };
+    const unsubscribers = shouldUnsubscribeSubscriptions.map((shouldUnsubscribe, i) =>
+      emitter.subscribe((eventNum) => {
+        const subscriptionResultsForEventNum = subscriptionResults[eventNum] ?? [];
+        if (!subscriptionResults[eventNum])
+          subscriptionResults[eventNum] = subscriptionResultsForEventNum;
+
+        subscriptionResultsForEventNum.push(i);
+
+        if (shouldUnsubscribe) unsubscribers[i]();
+      }),
+    );
+
+    for (let i = 0; i < numEventsToEmit; i += 1) emitEvent();
+
+    // There should be results for each event that was run
+    expect(
+      Object.keys(subscriptionResults)
+        .map((eventNumString) => parseInt(eventNumString, 10))
+        .sort(),
+    ).toEqual(eventNumArray);
+
+    // All should have run the first time
+    expect(subscriptionResults[0]).toEqual(
+      shouldUnsubscribeSubscriptions.map((_shouldUnsubscribe, i) => i),
+    );
+    eventNumArrayAfterUnsubscribing.forEach((eventNum) => {
+      // Only the `false` ones (didn't unsubscribe) should have run after the first time
+      expect(subscriptionResults[eventNum]).toEqual(
+        shouldUnsubscribeSubscriptions
+          .map((shouldUnsubscribe, i) => (shouldUnsubscribe ? undefined : i))
+          .filter((i) => i !== undefined),
+      );
+    });
+  });
+});

lib/platform-bible-utils/src/platform-event-emitter.model.ts

@@ -88,7 +88,10 @@ export default class PlatformEventEmitter<T> implements Dispose {
   protected emitFn(event: T) {
     this.assertNotDisposed();
 
-    this.subscriptions?.forEach((callback) => callback(event));
+    // Clone the subscriptions array before iterating over the callbacks so the callback index
+    // doesn't get messed up if someone subscribes or unsubscribes inside one of the callbacks
+    const emitCallbacks = [...(this.subscriptions ?? [])];
+    emitCallbacks.forEach((callback) => callback(event));
   }
 
   /** Check to make sure this emitter is not disposed. Throw if it is */

src/shared/models/network-object-status.service-model.ts

@@ -1,4 +1,7 @@
 import { NetworkObjectDetails } from '@shared/models/network-object.model';
+// Linked in TSDoc
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+import { isSubset } from 'platform-bible-utils';
 
 // Functions that are exposed through the network object
 export interface NetworkObjectStatusRemoteServiceType {
@@ -17,6 +20,10 @@ export interface NetworkObjectStatusServiceType extends NetworkObjectStatusRemot
   /**
    * Get a promise that resolves when a network object is registered or rejects if a timeout is hit
    *
+   * @param objectDetailsToMatch Subset of object details on the network object to wait for.
+   *   Compared to object details using {@link isSubset}
+   * @param timeoutInMS Max duration to wait for the network object. If not provided, it will wait
+   *   indefinitely
    * @returns Promise that either resolves to the {@link NetworkObjectDetails} for a network object
    *   once the network object is registered, or rejects if a timeout is provided and the timeout is
    *   reached before the network object is registered