Skip to content

delete services that implement the new FirebaseService interface #3454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/small-grapes-teach.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/component": patch
---

Correctly delete services that implement the new FirebaseService interface when calling provider.delete()
32 changes: 31 additions & 1 deletion packages-exp/app-exp/src/api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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', () => {
Expand Down
26 changes: 25 additions & 1 deletion packages/component/src/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(),
Expand All @@ -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()', () => {
Expand Down
14 changes: 9 additions & 5 deletions packages/component/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,16 @@ export class Provider<T extends Name> {
async delete(): Promise<void> {
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) // next services
// eslint-disable-next-line @typescript-eslint/no-explicit-any
.map(service => (service as any).delete())
]);
}

isComponentSet(): boolean {
Expand Down