From d69244d25f6670f596cf36be681c230b098af22f Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 3 Jun 2021 09:58:52 -0600 Subject: [PATCH 01/12] Fire onInit callback if instance is already available --- packages/component/src/provider.ts | 32 ++++++++++++++++------- packages/firestore/src/api/credentials.ts | 3 +++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index de01e815691..572917fd626 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -49,7 +49,7 @@ export class Provider { * @param identifier A provider can provide mulitple instances of a service * if this.component.multipleInstances is true. */ - get(identifier: string = DEFAULT_ENTRY_NAME): Promise { + get(identifier?: string): Promise { // if multipleInstances is not supported, use the default name const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); @@ -99,11 +99,11 @@ export class Provider { 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 { } 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( @@ -263,9 +263,21 @@ export class Provider { * * @returns a function to unregister the callback */ - onInit(callback: OnInitCallBack): () => void { + onInit( + callback: OnInitCallBack, + options?: { + identifier?: string; + optional?: false; + } + ): () => void { this.onInitCallbacks.add(callback); + const instance = this.getImmediate(options); + if (instance) { + const identifier = this.normalizeInstanceIdentifier(options?.identifier); + callback(instance, identifier); + } + return () => { this.onInitCallbacks.delete(callback); }; @@ -324,7 +336,9 @@ export class Provider { 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 { diff --git a/packages/firestore/src/api/credentials.ts b/packages/firestore/src/api/credentials.ts index 566e393b274..0a6403e3e08 100644 --- a/packages/firestore/src/api/credentials.ts +++ b/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 { From 257ad9ecabcbcfe85fe06ff9003df0cb8f4c7358 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 3 Jun 2021 10:47:30 -0600 Subject: [PATCH 02/12] Create cold-llamas-breathe.md --- .changeset/cold-llamas-breathe.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/cold-llamas-breathe.md diff --git a/.changeset/cold-llamas-breathe.md b/.changeset/cold-llamas-breathe.md new file mode 100644 index 00000000000..8166bdba576 --- /dev/null +++ b/.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. From 85f3677adb3c7fca94adc752e2c199fb4addd530 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 3 Jun 2021 10:48:52 -0600 Subject: [PATCH 03/12] Create cold-llamas-breathe.md --- cold-llamas-breathe.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 cold-llamas-breathe.md diff --git a/cold-llamas-breathe.md b/cold-llamas-breathe.md new file mode 100644 index 00000000000..8166bdba576 --- /dev/null +++ b/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. From ce5bac9fb5147763d73379bd9f4d00f0ce314d8e Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 3 Jun 2021 10:49:14 -0600 Subject: [PATCH 04/12] Delete cold-llamas-breathe.md --- cold-llamas-breathe.md | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 cold-llamas-breathe.md diff --git a/cold-llamas-breathe.md b/cold-llamas-breathe.md deleted file mode 100644 index 8166bdba576..00000000000 --- a/cold-llamas-breathe.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -"@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. From d83784216b894a4be9eceb552461c4550c9fcb0b Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Thu, 3 Jun 2021 11:21:38 -0600 Subject: [PATCH 05/12] Allow non-existing instance --- packages/component/src/provider.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 572917fd626..4b32d8dd6e9 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -263,19 +263,13 @@ export class Provider { * * @returns a function to unregister the callback */ - onInit( - callback: OnInitCallBack, - options?: { - identifier?: string; - optional?: false; - } - ): () => void { + onInit(callback: OnInitCallBack, identifier?: string): () => void { this.onInitCallbacks.add(callback); - const instance = this.getImmediate(options); + const instance = this.getImmediate({ identifier, optional: true }); if (instance) { - const identifier = this.normalizeInstanceIdentifier(options?.identifier); - callback(instance, identifier); + const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); + callback(instance, normalizedIdentifier); } return () => { From 1acbf4e9bf2dd0d688e2e9bedbb67bd6e1c1d8a7 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Mon, 7 Jun 2021 20:24:37 -0600 Subject: [PATCH 06/12] Feedback --- packages/component/src/provider.test.ts | 16 ++++++++++++++++ packages/component/src/provider.ts | 1 + 2 files changed, 17 insertions(+) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 13b14f8a42d..deaa2b68708 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -181,6 +181,22 @@ describe('Provider', () => { expect(callback2).to.have.been.calledOnce; }); + it('invokes callback for existing component', () => { + provider.setComponent( + getFakeComponent( + 'test', + () => ({ test: true }), + false, + InstantiationMode.EXPLICIT + ) + ); + const callback = fake(); + provider.initialize(); + provider.onInit(callback); + + expect(callback).to.have.been.calledOnce; + }); + it('returns a function to unregister the callback', () => { provider.setComponent( getFakeComponent( diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 4b32d8dd6e9..f32be0c563b 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -261,6 +261,7 @@ export class Provider { * @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, identifier?: string): () => void { From 9d5e14c166947eee6ad74400aad9d6189e0578c3 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 8 Jun 2021 15:45:15 -0600 Subject: [PATCH 07/12] Support multiple instances, add failing test --- packages/component/src/provider.test.ts | 28 +++++++++++++++++++++++-- packages/component/src/provider.ts | 26 +++++++++++++++-------- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index deaa2b68708..f83282936a0 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -197,6 +197,30 @@ describe('Provider', () => { expect(callback).to.have.been.calledOnce; }); + it('passes instance identifier', () => { + const component = { test: true }; + provider.setComponent( + getFakeComponent( + 'test', + () => component, + true, + InstantiationMode.EXPLICIT + ) + ); + const callback1 = fake(); + const callback2 = fake(); + + provider.initialize(); + + provider.onInit(callback1, 'id1'); + provider.onInit(callback2, 'id2'); + + expect(callback1).to.have.been.calledOnce; + expect(callback1).to.have.been.calledWith(component, 'id1'); + expect(callback2).to.have.been.calledOnce; + expect(callback2).to.have.been.calledWith(component, 'id1'); + }); + it('returns a function to unregister the callback', () => { provider.setComponent( getFakeComponent( @@ -209,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; diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index f32be0c563b..10100604a53 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -38,7 +38,7 @@ export class Provider { string, Deferred > = new Map(); - private onInitCallbacks: Set> = new Set(); + private onInitCallbacks: Map>> = new Map(); constructor( private readonly name: T, @@ -265,16 +265,20 @@ export class Provider { * @returns a function to unregister the callback */ onInit(callback: OnInitCallBack, identifier?: string): () => void { - this.onInitCallbacks.add(callback); - - const instance = this.getImmediate({ identifier, optional: true }); - if (instance) { - const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); - callback(instance, normalizedIdentifier); + const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); + const existingInstances = + this.onInitCallbacks.get(normalizedIdentifier) ?? + new Set>(); + existingInstances.add(callback); + this.onInitCallbacks.set(normalizedIdentifier, existingInstances); + + const existingInstance = this.instances.has(normalizedIdentifier); + if (existingInstance) { + callback(existingInstance, normalizedIdentifier); } return () => { - this.onInitCallbacks.delete(callback); + this.onInitCallbacks.get(normalizedIdentifier)!.delete(callback); }; } @@ -286,7 +290,11 @@ export class Provider { 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 { From 7b78632d6e8b49a4c147b9c52958882de2be131c Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Tue, 8 Jun 2021 20:22:47 -0600 Subject: [PATCH 08/12] Fix test --- packages/component/.run/All Tests.run.xml | 19 +++++++++++++++++++ packages/component/src/provider.test.ts | 14 +++++++------- 2 files changed, 26 insertions(+), 7 deletions(-) create mode 100644 packages/component/.run/All Tests.run.xml diff --git a/packages/component/.run/All Tests.run.xml b/packages/component/.run/All Tests.run.xml new file mode 100644 index 00000000000..78dc458e6c6 --- /dev/null +++ b/packages/component/.run/All Tests.run.xml @@ -0,0 +1,19 @@ + + + project + + $PROJECT_DIR$/../../node_modules/mocha + $PROJECT_DIR$ + true + + + + + + bdd + --require ts-node/register/type-check --require index.ts + PATTERN + src/**/*.test.ts + + + \ No newline at end of file diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index f83282936a0..445baefb02f 100644 --- a/packages/component/src/provider.test.ts +++ b/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 @@ -198,27 +198,27 @@ describe('Provider', () => { }); it('passes instance identifier', () => { - const component = { test: true }; provider.setComponent( getFakeComponent( 'test', - () => component, + () => ({ test: true }), true, - InstantiationMode.EXPLICIT + InstantiationMode.EAGER ) ); const callback1 = fake(); const callback2 = fake(); - provider.initialize(); + 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(component, 'id1'); + expect(callback1).to.have.been.calledWith(match.any, 'id1'); expect(callback2).to.have.been.calledOnce; - expect(callback2).to.have.been.calledWith(component, 'id1'); + expect(callback2).to.have.been.calledWith(match.any, 'id2'); }); it('returns a function to unregister the callback', () => { From 902f8304661e9c103eb36d8ef7805a7b608ee2eb Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 9 Jun 2021 09:30:02 -0600 Subject: [PATCH 09/12] Update packages/component/src/provider.test.ts Co-authored-by: Feiyang --- packages/component/src/provider.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 445baefb02f..4344aa75a56 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -181,7 +181,7 @@ describe('Provider', () => { expect(callback2).to.have.been.calledOnce; }); - it('invokes callback for existing component', () => { + it('invokes callback for existing instance', () => { provider.setComponent( getFakeComponent( 'test', From eaaecf7c4689069b391ae0dda2a9ac4afd4e7f85 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 9 Jun 2021 09:30:16 -0600 Subject: [PATCH 10/12] Update packages/component/src/provider.ts Co-authored-by: Feiyang --- packages/component/src/provider.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 10100604a53..ff19bde4112 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -266,11 +266,11 @@ export class Provider { */ onInit(callback: OnInitCallBack, identifier?: string): () => void { const normalizedIdentifier = this.normalizeInstanceIdentifier(identifier); - const existingInstances = + const existingCallbacks = this.onInitCallbacks.get(normalizedIdentifier) ?? new Set>(); - existingInstances.add(callback); - this.onInitCallbacks.set(normalizedIdentifier, existingInstances); + existingCallbacks.add(callback); + this.onInitCallbacks.set(normalizedIdentifier, existingCallbacks); const existingInstance = this.instances.has(normalizedIdentifier); if (existingInstance) { From 072cdaf8add674c1cdd7af9a3db48526cf057a91 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 9 Jun 2021 09:30:23 -0600 Subject: [PATCH 11/12] Update packages/component/src/provider.ts Co-authored-by: Feiyang --- packages/component/src/provider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index ff19bde4112..1eaad815e4e 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -278,7 +278,7 @@ export class Provider { } return () => { - this.onInitCallbacks.get(normalizedIdentifier)!.delete(callback); + existingCallbacks.delete(callback); }; } From 701ef077ae897db398f54f795013894bf3e08e05 Mon Sep 17 00:00:00 2001 From: Sebastian Schmidt Date: Wed, 9 Jun 2021 09:31:09 -0600 Subject: [PATCH 12/12] Delete All Tests.run.xml --- packages/component/.run/All Tests.run.xml | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 packages/component/.run/All Tests.run.xml diff --git a/packages/component/.run/All Tests.run.xml b/packages/component/.run/All Tests.run.xml deleted file mode 100644 index 78dc458e6c6..00000000000 --- a/packages/component/.run/All Tests.run.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - project - - $PROJECT_DIR$/../../node_modules/mocha - $PROJECT_DIR$ - true - - - - - - bdd - --require ts-node/register/type-check --require index.ts - PATTERN - src/**/*.test.ts - - - \ No newline at end of file