Skip to content

Commit b295a78

Browse files
committed
Address PR comments
1 parent 3871f26 commit b295a78

File tree

8 files changed

+21
-37
lines changed

8 files changed

+21
-37
lines changed

packages-exp/analytics-exp/src/api.test.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const fakeDynamicConfig = stub(
3535
});
3636

3737
// Fake indexedDB.open() request
38-
let fakeRequest = {
38+
const fakeRequest = {
3939
onsuccess: () => {},
4040
result: {
4141
close: () => {}
@@ -46,13 +46,7 @@ let idbOpenStub = stub();
4646
// Stub indexedDB.open() because sinon's clock does not know
4747
// how to wait for the real indexedDB callbacks to resolve.
4848
function stubIdbOpen(): void {
49-
(fakeRequest = {
50-
onsuccess: () => {},
51-
result: {
52-
close: () => {}
53-
}
54-
}),
55-
(idbOpenStub = stub(indexedDB, 'open').returns(fakeRequest as any));
49+
idbOpenStub = stub(indexedDB, 'open').returns(fakeRequest as any);
5650
}
5751

5852
describe('FirebaseAnalytics API tests', () => {
@@ -79,12 +73,6 @@ describe('FirebaseAnalytics API tests', () => {
7973
fakeDynamicConfig.restore();
8074
});
8175

82-
it('initializeAnalytics() returns an Analytics instance', async () => {
83-
app = getFullApp(fakeAppParams);
84-
const analyticsInstance = initializeAnalytics(app);
85-
expect(analyticsInstance.app).to.equal(app);
86-
await clock.runAllAsync();
87-
});
8876
it('initializeAnalytics() with same (no) options returns same instance', async () => {
8977
app = getFullApp(fakeAppParams);
9078
const analyticsInstance = initializeAnalytics(app);

packages-exp/analytics-exp/src/api.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,7 @@ export function initializeAnalytics(
9999
);
100100
if (analyticsProvider.isInitialized()) {
101101
const existingInstance = analyticsProvider.getImmediate();
102-
if (
103-
deepEqual(
104-
options.config || {},
105-
(analyticsProvider.getOptions() as AnalyticsSettings)?.config || {}
106-
)
107-
) {
102+
if (deepEqual(options, analyticsProvider.getOptions())) {
108103
return existingInstance;
109104
} else {
110105
throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED);

packages-exp/analytics-exp/src/errors.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ const ERRORS: ErrorMap<AnalyticsError> = {
3636
' already exists. ' +
3737
'Only one Firebase Analytics instance can be created for each appId.',
3838
[AnalyticsError.ALREADY_INITIALIZED]:
39-
'Firebase Analytics has already been initialized with these options. ' +
4039
'initializeAnalytics() cannot be called again with different options than those ' +
4140
'it was initially called with. It can be called again with the same options to ' +
4241
'return the existing instance, or getAnalytics() can be used ' +

packages-exp/analytics-exp/src/index.test.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ let clock: sinon.SinonFakeTimers;
4848
let fakeInstallations: _FirebaseInstallationsInternal;
4949

5050
// Fake indexedDB.open() request
51-
let fakeRequest = {
51+
const fakeRequest = {
5252
onsuccess: () => {},
5353
result: {
5454
close: () => {}
@@ -67,13 +67,7 @@ function stubFetch(status: number, body: object): void {
6767
// Stub indexedDB.open() because sinon's clock does not know
6868
// how to wait for the real indexedDB callbacks to resolve.
6969
function stubIdbOpen(): void {
70-
(fakeRequest = {
71-
onsuccess: () => {},
72-
result: {
73-
close: () => {}
74-
}
75-
}),
76-
(idbOpenStub = stub(indexedDB, 'open').returns(fakeRequest as any));
70+
idbOpenStub = stub(indexedDB, 'open').returns(fakeRequest as any);
7771
}
7872

7973
describe('FirebaseAnalytics instance tests', () => {

packages-exp/analytics-exp/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ declare global {
4343
}
4444
}
4545

46-
export function registerAnalytics(): void {
46+
function registerAnalytics(): void {
4747
_registerComponent(
4848
new Component(
4949
ANALYTICS_TYPE,

packages-exp/analytics-exp/testing/get-fake-firebase-services.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
} from '@firebase/app-exp';
2323
import { Component, ComponentType } from '@firebase/component';
2424
import { _FirebaseInstallationsInternal } from '@firebase/installations-exp';
25-
import { registerAnalytics } from '../src';
25+
import { AnalyticsService } from '../src/factory';
2626

2727
const fakeConfig = {
2828
projectId: 'projectId',
@@ -63,12 +63,20 @@ export function getFullApp(fakeAppParams?: {
6363
apiKey?: string;
6464
measurementId?: string;
6565
}): FirebaseApp {
66-
registerAnalytics();
6766
_registerComponent(
6867
new Component(
6968
'installations-exp-internal',
7069
() => {
71-
return getFakeInstallations();
70+
return {} as _FirebaseInstallationsInternal;
71+
},
72+
ComponentType.PUBLIC
73+
)
74+
);
75+
_registerComponent(
76+
new Component(
77+
'analytics-exp',
78+
() => {
79+
return {} as AnalyticsService;
7280
},
7381
ComponentType.PUBLIC
7482
)

packages-exp/app-check-exp/src/errors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ const ERRORS: ErrorMap<AppCheckError> = {
3333
[AppCheckError.ALREADY_INITIALIZED]:
3434
'You have already called initializeAppCheck() for FirebaseApp {$appName} with ' +
3535
'different options. To avoid this error, call initializeAppCheck() with the ' +
36-
'same options as when it was originally called, or call getAppCheck() to return the' +
37-
' already initialized instance.',
36+
'same options as when it was originally called. This will return the ' +
37+
'already initialized instance.',
3838
[AppCheckError.USE_BEFORE_ACTIVATION]:
3939
'App Check is being used before initializeAppCheck() is called for FirebaseApp {$appName}. ' +
4040
'Call initializeAppCheck() before instantiating other Firebase services.',

packages/component/src/provider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export class Provider<T extends Name> {
3838
string,
3939
Deferred<NameServiceMapping[T]>
4040
> = new Map();
41-
private readonly instancesOptions: Map<string, InitializeOptions> = new Map();
41+
private readonly instancesOptions: Map<string, Record<string, unknown>> = new Map();
4242
private onInitCallbacks: Map<string, Set<OnInitCallBack<T>>> = new Map();
4343

4444
constructor(
@@ -219,7 +219,7 @@ export class Provider<T extends Name> {
219219
return this.instances.has(identifier);
220220
}
221221

222-
getOptions(identifier: string = DEFAULT_ENTRY_NAME): InitializeOptions {
222+
getOptions(identifier: string = DEFAULT_ENTRY_NAME): Record<string, unknown> {
223223
return this.instancesOptions.get(identifier) || {};
224224
}
225225

0 commit comments

Comments
 (0)