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..d26649514d5 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/component/src/provider.test.ts b/packages/component/src/provider.test.ts index 253b9942f5b..98b48af2931 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..6ca123f5a14 100644 --- a/packages/component/src/provider.ts +++ b/packages/component/src/provider.ts @@ -170,12 +170,25 @@ 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 && + this.component && + // TODO: remove !== 'app' when modular SDKs become official + // People call app.delete() to trigger provider.delete() for all registered components, so + // we don't call delete() on legacy FirebaseApp to avoid getting into a loop. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (this.component.name as any) !== 'app' + ) // modular services + // eslint-disable-next-line @typescript-eslint/no-explicit-any + .map(service => (service as any).delete()) + ]); } isComponentSet(): boolean {