Skip to content

Commit

Permalink
Merge 79f0a00 into ed4f95c
Browse files Browse the repository at this point in the history
  • Loading branch information
schmidt-sebastian authored Jun 9, 2021
2 parents ed4f95c + 79f0a00 commit d3e02ac
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 16 deletions.
6 changes: 6 additions & 0 deletions .changeset/cold-llamas-breathe.md
Original file line number Diff line number Diff line change
@@ -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.
46 changes: 43 additions & 3 deletions packages/component/src/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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;
Expand Down
43 changes: 30 additions & 13 deletions packages/component/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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);

Expand Down Expand Up @@ -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) ||
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
};
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions packages/firestore/src/api/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit d3e02ac

Please sign in to comment.