Fire onInit callback if instance is already available (#4971)

Author: schmidt-sebastian

.changeset/cold-llamas-breathe.md

@@ -0,0 +1,6 @@
+---
+"@firebase/component": patch
+"@firebase/firestore": patch
+---
+
+Fixes a regression that prevented Firestore from detecting Auth during its initial initialization, which could cause some writes to not be send.

packages/component/src/provider.test.ts

@@ -16,7 +16,7 @@
  */
 
 import { expect } from 'chai';
-import { fake, SinonSpy } from 'sinon';
+import { fake, SinonSpy, match } from 'sinon';
 import { ComponentContainer } from './component_container';
 import { FirebaseService } from '@firebase/app-types/private';
 // eslint-disable-next-line import/no-extraneous-dependencies
@@ -181,6 +181,46 @@ describe('Provider', () => {
       expect(callback2).to.have.been.calledOnce;
     });
 
+    it('invokes callback for existing instance', () => {
+      provider.setComponent(
+        getFakeComponent(
+          'test',
+          () => ({ test: true }),
+          false,
+          InstantiationMode.EXPLICIT
+        )
+      );
+      const callback = fake();
+      provider.initialize();
+      provider.onInit(callback);
+
+      expect(callback).to.have.been.calledOnce;
+    });
+
+    it('passes instance identifier', () => {
+      provider.setComponent(
+        getFakeComponent(
+          'test',
+          () => ({ test: true }),
+          true,
+          InstantiationMode.EAGER
+        )
+      );
+      const callback1 = fake();
+      const callback2 = fake();
+
+      provider.getImmediate({ identifier: 'id1' });
+      provider.getImmediate({ identifier: 'id2' });
+
+      provider.onInit(callback1, 'id1');
+      provider.onInit(callback2, 'id2');
+
+      expect(callback1).to.have.been.calledOnce;
+      expect(callback1).to.have.been.calledWith(match.any, 'id1');
+      expect(callback2).to.have.been.calledOnce;
+      expect(callback2).to.have.been.calledWith(match.any, 'id2');
+    });
+
     it('returns a function to unregister the callback', () => {
       provider.setComponent(
         getFakeComponent(
@@ -193,8 +233,8 @@ describe('Provider', () => {
       const callback1 = fake();
       const callback2 = fake();
       provider.onInit(callback1);
-      const unregsiter = provider.onInit(callback2);
-      unregsiter();
+      const unregister = provider.onInit(callback2);
+      unregister();
 
       provider.initialize();
       expect(callback1).to.have.been.calledOnce;

packages/component/src/provider.ts

@@ -38,7 +38,7 @@ export class Provider<T extends Name> {
     string,
     Deferred<NameServiceMapping[T]>
   > = new Map();
-  private onInitCallbacks: Set<OnInitCallBack<T>> = new Set();
+  private onInitCallbacks: Map<string, Set<OnInitCallBack<T>>> = new Map();
 
   constructor(
     private readonly name: T,
@@ -49,7 +49,7 @@ export class Provider<T extends Name> {
    * @param identifier A provider can provide mulitple instances of a service
    * if this.component.multipleInstances is true.
    */
-  get(identifier: string = DEFAULT_ENTRY_NAME): Promise<NameServiceMapping[T]> {
+  get(identifier?: string): Promise<NameServiceMapping[T]> {
     // if multipleInstances is not supported, use the default name
     const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier);
 
@@ -99,11 +99,11 @@ export class Provider<T extends Name> {
     identifier?: string;
     optional?: boolean;
   }): NameServiceMapping[T] | null {
-    const identifier = options?.identifier ?? DEFAULT_ENTRY_NAME;
-    const optional = options?.optional ?? false;
-
     // if multipleInstances is not supported, use the default name
-    const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier);
+    const normalizedIdentifier = this.normalizeInstanceIdentifier(
+      options?.identifier
+    );
+    const optional = options?.optional ?? false;
 
     if (
       this.isInitialized(normalizedIdentifier) ||
@@ -219,9 +219,9 @@ export class Provider<T extends Name> {
   }
 
   initialize(opts: InitializeOptions = {}): NameServiceMapping[T] {
-    const { instanceIdentifier = DEFAULT_ENTRY_NAME, options = {} } = opts;
+    const { options = {} } = opts;
     const normalizedIdentifier = this.normalizeInstanceIdentifier(
-      instanceIdentifier
+      opts.instanceIdentifier
     );
     if (this.isInitialized(normalizedIdentifier)) {
       throw Error(
@@ -261,13 +261,24 @@ export class Provider<T extends Name> {
    * @param callback - a function that will be invoked  after the provider has been initialized by calling provider.initialize().
    * The function is invoked SYNCHRONOUSLY, so it should not execute any longrunning tasks in order to not block the program.
    *
+   * @param identifier An optional instance identifier
    * @returns a function to unregister the callback
    */
-  onInit(callback: OnInitCallBack<T>): () => void {
-    this.onInitCallbacks.add(callback);
+  onInit(callback: OnInitCallBack<T>, identifier?: string): () => void {
+    const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier);
+    const existingCallbacks =
+      this.onInitCallbacks.get(normalizedIdentifier) ??
+      new Set<OnInitCallBack<T>>();
+    existingCallbacks.add(callback);
+    this.onInitCallbacks.set(normalizedIdentifier, existingCallbacks);
+
+    const existingInstance = this.instances.has(normalizedIdentifier);
+    if (existingInstance) {
+      callback(existingInstance, normalizedIdentifier);
+    }
 
     return () => {
-      this.onInitCallbacks.delete(callback);
+      existingCallbacks.delete(callback);
     };
   }
 
@@ -279,7 +290,11 @@ export class Provider<T extends Name> {
     instance: NameServiceMapping[T],
     identifier: string
   ): void {
-    for (const callback of this.onInitCallbacks) {
+    const callbacks = this.onInitCallbacks.get(identifier);
+    if (!callbacks) {
+      return;
+    }
+    for (const callback of callbacks) {
       try {
         callback(instance, identifier);
       } catch {
@@ -324,7 +339,9 @@ export class Provider<T extends Name> {
     return instance || null;
   }
 
-  private normalizeInstanceIdentifier(identifier: string): string {
+  private normalizeInstanceIdentifier(
+    identifier: string = DEFAULT_ENTRY_NAME
+  ): string {
     if (this.component) {
       return this.component.multipleInstances ? identifier : DEFAULT_ENTRY_NAME;
     } else {

packages/firestore/src/api/credentials.ts

@@ -307,6 +307,9 @@ export class FirebaseCredentialsProvider implements CredentialsProvider {
     this.invokeChangeListener = true;
     this.asyncQueue = asyncQueue;
     this.changeListener = changeListener;
+    if (this.auth) {
+      this.awaitTokenAndRaiseInitialEvent();
+    }
   }
 
   removeChangeListener(): void {