diff --git a/.changeset/small-grapes-teach.md b/.changeset/small-grapes-teach.md new file mode 100644 index 00000000000..5a15dfe3552 --- /dev/null +++ b/.changeset/small-grapes-teach.md @@ -0,0 +1,5 @@ +--- +"@firebase/component": patch +--- + +Correctly delete services created by modular SDKs when calling provider.delete() diff --git a/packages-exp/app-exp/src/api.test.ts b/packages-exp/app-exp/src/api.test.ts index 307ab4b621f..5d5c9f99e0a 100644 --- a/packages-exp/app-exp/src/api.test.ts +++ b/packages-exp/app-exp/src/api.test.ts @@ -29,7 +29,10 @@ import { onLog } from './api'; import { DEFAULT_ENTRY_NAME } from './constants'; -import { _FirebaseAppInternal } from '@firebase/app-types-exp'; +import { + _FirebaseAppInternal, + _FirebaseService +} from '@firebase/app-types-exp'; import { _clearComponents, _components, @@ -175,6 +178,33 @@ describe('API tests', () => { deleteApp(app).catch(() => {}); expect(getApps().length).to.equal(0); }); + + it('waits for all services being deleted', async () => { + _clearComponents(); + let count = 0; + const comp1 = new Component( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + 'test1' as any, + _container => + ({ + _delete: async () => { + await Promise.resolve(); + expect(count).to.equal(0); + count++; + } + } as _FirebaseService), + ComponentType.PUBLIC + ); + _registerComponent(comp1); + + const app = initializeApp({}); + // create service instance + const test1Provider = _getProvider(app, 'test1' as any); + test1Provider.getImmediate(); + + await deleteApp(app); + expect(count).to.equal(1); + }); }); describe('registerVersion', () => { diff --git a/packages-exp/app-exp/test/util.ts b/packages-exp/app-exp/test/util.ts index 5de89e70294..bc41c08fbca 100644 --- a/packages-exp/app-exp/test/util.ts +++ b/packages-exp/app-exp/test/util.ts @@ -25,7 +25,7 @@ export class TestService implements _FirebaseService { return this.app_; } - delete(): Promise { + _delete(): Promise { return new Promise((resolve: (v?: void) => void) => { setTimeout(() => resolve(), 10); }); diff --git a/packages-exp/app-types-exp/index.d.ts b/packages-exp/app-types-exp/index.d.ts index 91e4a57c534..d74be71601c 100644 --- a/packages-exp/app-types-exp/index.d.ts +++ b/packages-exp/app-types-exp/index.d.ts @@ -113,7 +113,7 @@ export interface _FirebaseService { * Delete the service and free it's resources - called from * {@link @firebase/app-exp#deleteApp | deleteApp()} */ - delete(): Promise; + _delete(): Promise; } export interface VersionService { diff --git a/packages-exp/functions-exp/src/service.ts b/packages-exp/functions-exp/src/service.ts index 9505ed4377f..233896842fb 100644 --- a/packages-exp/functions-exp/src/service.ts +++ b/packages-exp/functions-exp/src/service.ts @@ -95,7 +95,7 @@ export class FunctionsService implements _FirebaseService { }); } - delete(): Promise { + _delete(): Promise { return this.deleteService(); } diff --git a/packages/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 253b9942f5b..cd321a2b522 100644 --- a/packages/component/src/provider.test.ts +++ b/packages/component/src/provider.test.ts @@ -19,6 +19,7 @@ import { expect } from 'chai'; import { fake, SinonSpy } from 'sinon'; import { ComponentContainer } from './component_container'; import { FirebaseService } from '@firebase/app-types/private'; +import { _FirebaseService } from '@firebase/app-types-exp'; import { Provider } from './provider'; import { getFakeApp, getFakeComponent } from '../test/util'; import '../test/setup'; @@ -202,7 +203,7 @@ describe('Provider', () => { }); describe('delete()', () => { - it('calls delete() on the service instance that implements FirebaseService', () => { + it('calls delete() on the service instance that implements legacy FirebaseService', () => { const deleteFake = fake(); const myService: FirebaseService = { app: getFakeApp(), @@ -226,6 +227,29 @@ describe('Provider', () => { expect(deleteFake).to.have.been.called; }); + + it('calls delete() on the service instance that implements next FirebaseService', () => { + const deleteFake = fake(); + const myService: _FirebaseService = { + app: getFakeApp(), + _delete: deleteFake + }; + + // provide factory and create a service instance + provider.setComponent( + getFakeComponent( + 'test', + () => myService, + false, + InstantiationMode.EAGER + ) + ); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + provider.delete(); + + expect(deleteFake).to.have.been.called; + }); }); describe('clearCache()', () => { diff --git a/packages/component/src/provider.ts b/packages/component/src/provider.ts index 14bd0de092a..a3b7d809806 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -170,12 +170,16 @@ export class Provider { async delete(): Promise { const services = Array.from(this.instances.values()); - await Promise.all( - services - .filter(service => 'INTERNAL' in service) + await Promise.all([ + ...services + .filter(service => 'INTERNAL' in service) // legacy services // eslint-disable-next-line @typescript-eslint/no-explicit-any - .map(service => (service as any).INTERNAL!.delete()) - ); + .map(service => (service as any).INTERNAL!.delete()), + ...services + .filter(service => '_delete' in service) // modularized services + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .map(service => (service as any)._delete()) + ]); } isComponentSet(): boolean { diff --git a/packages/firestore/exp/src/api/components.ts b/packages/firestore/exp/src/api/components.ts index 615d68176e1..fb043129cee 100644 --- a/packages/firestore/exp/src/api/components.ts +++ b/packages/firestore/exp/src/api/components.ts @@ -23,7 +23,7 @@ import { OfflineComponentProvider, OnlineComponentProvider } from '../../../src/core/component_provider'; -import {handleUserChange, LocalStore} from '../../../src/local/local_store'; +import { handleUserChange, LocalStore } from '../../../src/local/local_store'; import { Deferred } from '../../../src/util/promise'; import { logDebug } from '../../../src/util/log'; import { SyncEngine } from '../../../src/core/sync_engine'; @@ -71,7 +71,7 @@ export async function setOfflineComponentProvider( // When a user calls clearPersistence() in one client, all other clients // need to be terminated to allow the delete to succeed. offlineComponentProvider.persistence.setDatabaseDeletedListener(() => - firestore.delete() + firestore._delete() ); offlineDeferred.resolve(offlineComponentProvider); } diff --git a/packages/firestore/exp/src/api/database.ts b/packages/firestore/exp/src/api/database.ts index 1b186ea8075..fece5ab481a 100644 --- a/packages/firestore/exp/src/api/database.ts +++ b/packages/firestore/exp/src/api/database.ts @@ -69,7 +69,8 @@ const LOG_TAG = 'Firestore'; * The root reference to the Firestore database and the entry point for the * tree-shakeable SDK. */ -export class Firestore extends LiteFirestore +export class Firestore + extends LiteFirestore implements firestore.FirebaseFirestore, _FirebaseService { readonly _queue = new AsyncQueue(); readonly _persistenceKey: string; @@ -320,7 +321,7 @@ export function terminate( ): Promise { _removeServiceInstance(firestore.app, 'firestore-exp'); const firestoreImpl = cast(firestore, Firestore); - return firestoreImpl.delete(); + return firestoreImpl._delete(); } function verifyNotInitialized(firestore: Firestore): void { diff --git a/packages/firestore/lite/src/api/database.ts b/packages/firestore/lite/src/api/database.ts index 7f4c800cca3..6945397f4ac 100644 --- a/packages/firestore/lite/src/api/database.ts +++ b/packages/firestore/lite/src/api/database.ts @@ -95,7 +95,7 @@ export class Firestore return new DatabaseId(app.options.projectId!); } - delete(): Promise { + _delete(): Promise { if (!this._terminateTask) { this._terminateTask = this._terminate(); } @@ -117,7 +117,7 @@ export class Firestore // TODO(firestoreexp): `deleteApp()` should call the delete method above, // but it still calls INTERNAL.delete(). INTERNAL = { - delete: () => this.delete() + delete: () => this._delete() }; } @@ -142,5 +142,5 @@ export function terminate( ): Promise { _removeServiceInstance(firestore.app, 'firestore/lite'); const firestoreClient = cast(firestore, Firestore); - return firestoreClient.delete(); + return firestoreClient._delete(); }