From 4c7ae3d20aca6ab56cfda99616a3b61c33355c87 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 14:23:19 -0800 Subject: [PATCH 1/6] Get installations from the container --- packages/analytics/index.test.ts | 18 ++++++++++++------ packages/analytics/index.ts | 5 ++++- packages/analytics/src/factory.ts | 6 +++--- packages/analytics/src/helpers.test.ts | 7 ++++--- packages/analytics/src/helpers.ts | 5 +++-- ...ke-app.ts => get-fake-firebase-services.ts} | 18 +++++++++++++----- packages/firebase/rollup.config.js | 2 +- 7 files changed, 40 insertions(+), 21 deletions(-) rename packages/analytics/testing/{get-fake-app.ts => get-fake-firebase-services.ts} (72%) diff --git a/packages/analytics/index.test.ts b/packages/analytics/index.test.ts index 5c9761a3a52..83620ae976c 100644 --- a/packages/analytics/index.test.ts +++ b/packages/analytics/index.test.ts @@ -24,7 +24,7 @@ import { factory as analyticsFactory, resetGlobalVars } from './index'; -import { getFakeApp } from './testing/get-fake-app'; +import { getFakeApp, getFakeInstallations } from './testing/get-fake-firebase-services'; import { FirebaseApp } from '@firebase/app-types'; import { GtagCommand, EventName } from './src/constants'; import { findGtagScriptOnPage } from './src/helpers'; @@ -39,21 +39,25 @@ const customDataLayerName = 'customDataLayer'; describe('FirebaseAnalytics instance tests', () => { it('Throws if no analyticsId in config', () => { const app = getFakeApp(); - expect(() => analyticsFactory(app)).to.throw('field is empty'); + const installations = getFakeInstallations(); + expect(() => analyticsFactory(app, installations)).to.throw('field is empty'); }); it('Throws if creating an instance with already-used analytics ID', () => { const app = getFakeApp(analyticsId); + const installations = getFakeInstallations(); resetGlobalVars(false, { [analyticsId]: Promise.resolve() }); - expect(() => analyticsFactory(app)).to.throw('already exists'); + expect(() => analyticsFactory(app, installations)).to.throw('already exists'); }); describe('Standard app, page already has user gtag script', () => { let app: FirebaseApp = {} as FirebaseApp; before(() => { resetGlobalVars(); app = getFakeApp(analyticsId); + const installations = getFakeInstallations(); + window['gtag'] = gtagStub; window['dataLayer'] = []; - analyticsInstance = analyticsFactory(app); + analyticsInstance = analyticsFactory(app, installations); }); after(() => { delete window['gtag']; @@ -113,13 +117,14 @@ describe('FirebaseAnalytics instance tests', () => { before(() => { resetGlobalVars(); const app = getFakeApp(analyticsId); + const installations = getFakeInstallations(); window[customGtagName] = gtagStub; window[customDataLayerName] = []; analyticsSettings({ dataLayerName: customDataLayerName, gtagName: customGtagName }); - analyticsInstance = analyticsFactory(app); + analyticsInstance = analyticsFactory(app, installations); }); after(() => { delete window[customGtagName]; @@ -162,7 +167,8 @@ describe('FirebaseAnalytics instance tests', () => { before(() => { resetGlobalVars(); const app = getFakeApp(analyticsId); - analyticsInstance = analyticsFactory(app); + const installations = getFakeInstallations(); + analyticsInstance = analyticsFactory(app, installations); }); after(() => { delete window['gtag']; diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index 1f094900a53..4613224ee9e 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -15,6 +15,7 @@ * limitations under the License. */ import firebase from '@firebase/app'; +import '@firebase/installations'; import { FirebaseAnalytics } from '@firebase/analytics-types'; import { FirebaseAnalyticsInternal } from '@firebase/analytics-interop-types'; import { _FirebaseNamespace } from '@firebase/app-types/private'; @@ -44,7 +45,9 @@ export function registerAnalytics(instance: _FirebaseNamespace): void { container => { // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app').getImmediate(); - return factory(app); + const installations = container.getProvider('installations').getImmediate(); + + return factory(app, installations); }, ComponentType.PUBLIC ).setServiceProps({ diff --git a/packages/analytics/src/factory.ts b/packages/analytics/src/factory.ts index 7f58f7e36c9..7b6a8f8954c 100644 --- a/packages/analytics/src/factory.ts +++ b/packages/analytics/src/factory.ts @@ -27,7 +27,6 @@ import { setUserProperties, setAnalyticsCollectionEnabled } from './functions'; -import '@firebase/installations'; import { initializeGAId, insertScriptTag, @@ -38,6 +37,7 @@ import { import { ANALYTICS_ID_FIELD } from './constants'; import { AnalyticsError, ERROR_FACTORY } from './errors'; import { FirebaseApp } from '@firebase/app-types'; +import { FirebaseInstallations } from '@firebase/installations-types'; /** * Maps gaId to FID fetch promises. @@ -102,7 +102,7 @@ export function settings(options: SettingsOptions): void { } } -export function factory(app: FirebaseApp): FirebaseAnalytics { +export function factory(app: FirebaseApp, installations: FirebaseInstallations): FirebaseAnalytics { const analyticsId = app.options[ANALYTICS_ID_FIELD]; if (!analyticsId) { throw ERROR_FACTORY.create(AnalyticsError.NO_GA_ID); @@ -135,7 +135,7 @@ export function factory(app: FirebaseApp): FirebaseAnalytics { globalInitDone = true; } // Async but non-blocking. - initializedIdPromisesMap[analyticsId] = initializeGAId(app, gtagCoreFunction); + initializedIdPromisesMap[analyticsId] = initializeGAId(app, installations, gtagCoreFunction); const analyticsInstance: FirebaseAnalytics = { app, diff --git a/packages/analytics/src/helpers.test.ts b/packages/analytics/src/helpers.test.ts index 9ded80d9a0a..89b36ef26ad 100644 --- a/packages/analytics/src/helpers.test.ts +++ b/packages/analytics/src/helpers.test.ts @@ -26,7 +26,7 @@ import { wrapOrCreateGtag, findGtagScriptOnPage } from './helpers'; -import { getFakeApp } from '../testing/get-fake-app'; +import { getFakeApp, getFakeInstallations } from '../testing/get-fake-firebase-services'; import { GtagCommand } from './constants'; import { Deferred } from '@firebase/util'; @@ -36,8 +36,9 @@ const mockFid = 'fid-1234-zyxw'; describe('FirebaseAnalytics methods', () => { it('initializeGAId gets FID from installations and calls gtag config with it', async () => { const gtagStub: SinonStub = stub(); - const app = getFakeApp(mockAnalyticsId, mockFid); - await initializeGAId(app, gtagStub); + const app = getFakeApp(mockAnalyticsId); + const installations = getFakeInstallations(mockFid); + await initializeGAId(app, installations, gtagStub); expect(gtagStub).to.be.calledWith(GtagCommand.CONFIG, mockAnalyticsId, { 'firebase_id': mockFid, 'origin': 'firebase', diff --git a/packages/analytics/src/helpers.ts b/packages/analytics/src/helpers.ts index eb7fb429adc..e0583611434 100644 --- a/packages/analytics/src/helpers.ts +++ b/packages/analytics/src/helpers.ts @@ -30,7 +30,7 @@ import { ORIGIN_KEY, GTAG_URL } from './constants'; -import '@firebase/installations'; +import { FirebaseInstallations } from '@firebase/installations-types'; /** * Initialize the analytics instance in gtag.js by calling config command with fid. @@ -42,9 +42,10 @@ import '@firebase/installations'; */ export async function initializeGAId( app: FirebaseApp, + installations: FirebaseInstallations, gtagCore: Gtag ): Promise { - const fid = await app.installations().getId(); + const fid = await installations.getId(); // This command initializes gtag.js and only needs to be called once for the entire web app, // but since it is idempotent, we can call it multiple times. diff --git a/packages/analytics/testing/get-fake-app.ts b/packages/analytics/testing/get-fake-firebase-services.ts similarity index 72% rename from packages/analytics/testing/get-fake-app.ts rename to packages/analytics/testing/get-fake-firebase-services.ts index 44d1a129ded..e4674ccafbf 100644 --- a/packages/analytics/testing/get-fake-app.ts +++ b/packages/analytics/testing/get-fake-firebase-services.ts @@ -16,11 +16,10 @@ */ import { FirebaseApp } from '@firebase/app-types'; -import { stub } from 'sinon'; +import { FirebaseInstallations } from '@firebase/installations-types'; export function getFakeApp( - measurementId?: string, - fid: string = 'fid-1234' + measurementId?: string ): FirebaseApp { return { name: 'appName', @@ -35,9 +34,18 @@ export function getFakeApp( measurementId }, automaticDataCollectionEnabled: true, - delete: async () => {}, - installations: stub().returns({ getId: () => Promise.resolve(fid) }), + delete: async () => { }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + installations: null as any, // eslint-disable-next-line @typescript-eslint/no-explicit-any analytics: null as any }; } + +export function getFakeInstallations(fid: string = 'fid-1234'): FirebaseInstallations { + return { + getId: () => Promise.resolve(fid), + getToken: () => Promise.resolve('authToken'), + delete: () => Promise.resolve() + }; +} diff --git a/packages/firebase/rollup.config.js b/packages/firebase/rollup.config.js index a98b2a6d7d4..212c11271e8 100644 --- a/packages/firebase/rollup.config.js +++ b/packages/firebase/rollup.config.js @@ -129,7 +129,7 @@ const componentBuilds = pkg.components ); }` }, - plugins: [...plugins, uglify()], + plugins: [...plugins], external: ['@firebase/app'] } ]; From 4fe416b6d3d4ffb7fcf7ef267ff0dc6971827c50 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 14:23:59 -0800 Subject: [PATCH 2/6] [AUTOMATED]: Prettier Code Styling --- packages/analytics/index.test.ts | 13 ++++++++++--- packages/analytics/index.ts | 4 +++- packages/analytics/src/factory.ts | 11 +++++++++-- packages/analytics/src/helpers.test.ts | 5 ++++- .../analytics/testing/get-fake-firebase-services.ts | 10 +++++----- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/packages/analytics/index.test.ts b/packages/analytics/index.test.ts index 83620ae976c..32ae96d6324 100644 --- a/packages/analytics/index.test.ts +++ b/packages/analytics/index.test.ts @@ -24,7 +24,10 @@ import { factory as analyticsFactory, resetGlobalVars } from './index'; -import { getFakeApp, getFakeInstallations } from './testing/get-fake-firebase-services'; +import { + getFakeApp, + getFakeInstallations +} from './testing/get-fake-firebase-services'; import { FirebaseApp } from '@firebase/app-types'; import { GtagCommand, EventName } from './src/constants'; import { findGtagScriptOnPage } from './src/helpers'; @@ -40,13 +43,17 @@ describe('FirebaseAnalytics instance tests', () => { it('Throws if no analyticsId in config', () => { const app = getFakeApp(); const installations = getFakeInstallations(); - expect(() => analyticsFactory(app, installations)).to.throw('field is empty'); + expect(() => analyticsFactory(app, installations)).to.throw( + 'field is empty' + ); }); it('Throws if creating an instance with already-used analytics ID', () => { const app = getFakeApp(analyticsId); const installations = getFakeInstallations(); resetGlobalVars(false, { [analyticsId]: Promise.resolve() }); - expect(() => analyticsFactory(app, installations)).to.throw('already exists'); + expect(() => analyticsFactory(app, installations)).to.throw( + 'already exists' + ); }); describe('Standard app, page already has user gtag script', () => { let app: FirebaseApp = {} as FirebaseApp; diff --git a/packages/analytics/index.ts b/packages/analytics/index.ts index 4613224ee9e..f99c16d93c4 100644 --- a/packages/analytics/index.ts +++ b/packages/analytics/index.ts @@ -45,7 +45,9 @@ export function registerAnalytics(instance: _FirebaseNamespace): void { container => { // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app').getImmediate(); - const installations = container.getProvider('installations').getImmediate(); + const installations = container + .getProvider('installations') + .getImmediate(); return factory(app, installations); }, diff --git a/packages/analytics/src/factory.ts b/packages/analytics/src/factory.ts index 7b6a8f8954c..b7fffe5a7db 100644 --- a/packages/analytics/src/factory.ts +++ b/packages/analytics/src/factory.ts @@ -102,7 +102,10 @@ export function settings(options: SettingsOptions): void { } } -export function factory(app: FirebaseApp, installations: FirebaseInstallations): FirebaseAnalytics { +export function factory( + app: FirebaseApp, + installations: FirebaseInstallations +): FirebaseAnalytics { const analyticsId = app.options[ANALYTICS_ID_FIELD]; if (!analyticsId) { throw ERROR_FACTORY.create(AnalyticsError.NO_GA_ID); @@ -135,7 +138,11 @@ export function factory(app: FirebaseApp, installations: FirebaseInstallations): globalInitDone = true; } // Async but non-blocking. - initializedIdPromisesMap[analyticsId] = initializeGAId(app, installations, gtagCoreFunction); + initializedIdPromisesMap[analyticsId] = initializeGAId( + app, + installations, + gtagCoreFunction + ); const analyticsInstance: FirebaseAnalytics = { app, diff --git a/packages/analytics/src/helpers.test.ts b/packages/analytics/src/helpers.test.ts index 89b36ef26ad..50a9f77d431 100644 --- a/packages/analytics/src/helpers.test.ts +++ b/packages/analytics/src/helpers.test.ts @@ -26,7 +26,10 @@ import { wrapOrCreateGtag, findGtagScriptOnPage } from './helpers'; -import { getFakeApp, getFakeInstallations } from '../testing/get-fake-firebase-services'; +import { + getFakeApp, + getFakeInstallations +} from '../testing/get-fake-firebase-services'; import { GtagCommand } from './constants'; import { Deferred } from '@firebase/util'; diff --git a/packages/analytics/testing/get-fake-firebase-services.ts b/packages/analytics/testing/get-fake-firebase-services.ts index e4674ccafbf..12d4fae1aca 100644 --- a/packages/analytics/testing/get-fake-firebase-services.ts +++ b/packages/analytics/testing/get-fake-firebase-services.ts @@ -18,9 +18,7 @@ import { FirebaseApp } from '@firebase/app-types'; import { FirebaseInstallations } from '@firebase/installations-types'; -export function getFakeApp( - measurementId?: string -): FirebaseApp { +export function getFakeApp(measurementId?: string): FirebaseApp { return { name: 'appName', options: { @@ -34,7 +32,7 @@ export function getFakeApp( measurementId }, automaticDataCollectionEnabled: true, - delete: async () => { }, + delete: async () => {}, // eslint-disable-next-line @typescript-eslint/no-explicit-any installations: null as any, // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -42,7 +40,9 @@ export function getFakeApp( }; } -export function getFakeInstallations(fid: string = 'fid-1234'): FirebaseInstallations { +export function getFakeInstallations( + fid: string = 'fid-1234' +): FirebaseInstallations { return { getId: () => Promise.resolve(fid), getToken: () => Promise.resolve('authToken'), From 4bbeacfa4a65808710d17d227b92364a51c66145 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 14:29:13 -0800 Subject: [PATCH 3/6] revert dev changes --- packages/firebase/rollup.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/firebase/rollup.config.js b/packages/firebase/rollup.config.js index 212c11271e8..a98b2a6d7d4 100644 --- a/packages/firebase/rollup.config.js +++ b/packages/firebase/rollup.config.js @@ -129,7 +129,7 @@ const componentBuilds = pkg.components ); }` }, - plugins: [...plugins], + plugins: [...plugins, uglify()], external: ['@firebase/app'] } ]; From 2247969fd659e500dc1f0fd15e0dddc5818828c0 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 15:18:10 -0800 Subject: [PATCH 4/6] convert eslint from json to js. --- packages/component/.eslintrc.js | 6 ++++++ packages/component/.eslintrc.json | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) create mode 100644 packages/component/.eslintrc.js delete mode 100644 packages/component/.eslintrc.json diff --git a/packages/component/.eslintrc.js b/packages/component/.eslintrc.js new file mode 100644 index 00000000000..19baa714ac6 --- /dev/null +++ b/packages/component/.eslintrc.js @@ -0,0 +1,6 @@ +module.exports = { + extends: "../../config/.eslintrc.js", + parserOptions: { + project: "tsconfig.json" + } +}; \ No newline at end of file diff --git a/packages/component/.eslintrc.json b/packages/component/.eslintrc.json deleted file mode 100644 index 2b39e9f6f85..00000000000 --- a/packages/component/.eslintrc.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "extends": "../../config/.eslintrc.json", - "parserOptions": { - "project": "tsconfig.json" - } -} \ No newline at end of file From bfb30b8a249fd39e3236b17630c8af3ee5004340 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 15:18:26 -0800 Subject: [PATCH 5/6] [AUTOMATED]: Prettier Code Styling --- packages/component/.eslintrc.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/component/.eslintrc.js b/packages/component/.eslintrc.js index 19baa714ac6..407df78bdb9 100644 --- a/packages/component/.eslintrc.js +++ b/packages/component/.eslintrc.js @@ -1,6 +1,6 @@ module.exports = { - extends: "../../config/.eslintrc.js", - parserOptions: { - project: "tsconfig.json" - } -}; \ No newline at end of file + extends: '../../config/.eslintrc.js', + parserOptions: { + project: 'tsconfig.json' + } +}; From a75c02e733fab4cb59e996c4b457d2f2b2003002 Mon Sep 17 00:00:00 2001 From: Feiyang1 Date: Fri, 22 Nov 2019 16:21:16 -0800 Subject: [PATCH 6/6] fix broken scripts --- packages/component/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/component/package.json b/packages/component/package.json index 7878ec24ce8..b680dfb5c81 100644 --- a/packages/component/package.json +++ b/packages/component/package.json @@ -12,8 +12,8 @@ "dist" ], "scripts": { - "lint": "eslint -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'", - "lint:fix": "eslint --fix -c .eslintrc.json '**/*.ts' --ignore-path '../../.gitignore'", + "lint": "eslint -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'", + "lint:fix": "eslint --fix -c .eslintrc.js '**/*.ts' --ignore-path '../../.gitignore'", "build": "rollup -c", "dev": "rollup -c -w", "test": "run-p lint test:browser test:node",