From fcb79cd2c572dc28e1c509aca4bf1049bcbbf2a7 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Thu, 25 Feb 2021 16:05:52 -0800 Subject: [PATCH 1/7] Add initializeAnalytics() --- packages-exp/analytics-exp/src/api.ts | 37 +++++++++++++++++++ packages-exp/analytics-exp/src/factory.ts | 32 ++++++++++------ packages-exp/analytics-exp/src/index.test.ts | 37 +++++++++++-------- packages-exp/analytics-exp/src/index.ts | 7 ++-- .../src/initialize-analytics.test.ts | 10 ++--- .../analytics-exp/src/initialize-analytics.ts | 18 +++++---- .../analytics-exp/src/public-types.ts | 30 +++++++++++++++ 7 files changed, 127 insertions(+), 44 deletions(-) diff --git a/packages-exp/analytics-exp/src/api.ts b/packages-exp/analytics-exp/src/api.ts index e99332aacae..e44d8f71a89 100644 --- a/packages-exp/analytics-exp/src/api.ts +++ b/packages-exp/analytics-exp/src/api.ts @@ -21,6 +21,7 @@ import { _getProvider, FirebaseApp, getApp } from '@firebase/app-exp'; import { Analytics, AnalyticsCallOptions, + AnalyticsOptions, CustomParams, EventNameString, EventParams @@ -37,6 +38,7 @@ import { ANALYTICS_TYPE } from './constants'; import { AnalyticsService, initializationPromisesMap, + _initializeAnalyticsForApp, wrappedGtagFunction } from './factory'; import { logger } from './logger'; @@ -70,7 +72,42 @@ export function getAnalytics(app: FirebaseApp = getApp()): Analytics { app, ANALYTICS_TYPE ); + + if (analyticsProvider.isInitialized()) { + return analyticsProvider.getImmediate(); + } + + return initializeAnalytics(app); +} + +/** + * Returns a Firebase Analytics instance for the given app. + * + * @public + * + * @param app - The FirebaseApp to use. + */ +export function initializeAnalytics( + app: FirebaseApp, + options?: AnalyticsOptions +): Analytics { + // Dependencies + const analyticsProvider: Provider<'analytics-exp'> = _getProvider( + app, + ANALYTICS_TYPE + ); + if (analyticsProvider.isInitialized()) { + return analyticsProvider.getImmediate(); + // TODO: Throw an analytics specific error. + // _fail(analyticsInstance, AuthErrorCode.ALREADY_INITIALIZED); + } const analyticsInstance = analyticsProvider.getImmediate(); + // do init settings stuff here + const installations = _getProvider( + app, + 'installations-exp-internal' + ).getImmediate(); + _initializeAnalyticsForApp(app, installations, options); return analyticsInstance; } diff --git a/packages-exp/analytics-exp/src/factory.ts b/packages-exp/analytics-exp/src/factory.ts index 4d12ad5c5be..cb47074af16 100644 --- a/packages-exp/analytics-exp/src/factory.ts +++ b/packages-exp/analytics-exp/src/factory.ts @@ -15,13 +15,13 @@ * limitations under the License. */ -import { SettingsOptions, Analytics } from './public-types'; +import { SettingsOptions, Analytics, AnalyticsOptions } from './public-types'; import { Gtag, DynamicConfig, MinimalDynamicConfig } from './types'; import { getOrCreateDataLayer, wrapOrCreateGtag } from './helpers'; import { AnalyticsError, ERROR_FACTORY } from './errors'; import { _FirebaseInstallationsInternal } from '@firebase/installations-exp'; import { areCookiesEnabled, isBrowserExtension } from '@firebase/util'; -import { initializeAnalytics } from './initialize-analytics'; +import { _initializeAnalytics } from './initialize-analytics'; import { logger } from './logger'; import { FirebaseApp, _FirebaseService } from '@firebase/app-exp'; @@ -174,10 +174,7 @@ function warnOnBrowserContextMismatch(): void { * Analytics instance factory. * @internal */ -export function factory( - app: FirebaseApp, - installations: _FirebaseInstallationsInternal -): AnalyticsService { +export function factory(app: FirebaseApp): AnalyticsService { warnOnBrowserContextMismatch(); const appId = app.options.appId; if (!appId) { @@ -218,18 +215,29 @@ export function factory( globalInitDone = true; } + + const analyticsInstance: AnalyticsService = new AnalyticsService(app); + return analyticsInstance; +} + +/** + * Starts async initializations for this app. + */ +export function _initializeAnalyticsForApp( + app: FirebaseApp, + installations: _FirebaseInstallationsInternal, + options?: AnalyticsOptions +): void { // Async but non-blocking. // This map reflects the completion state of all promises for each appId. - initializationPromisesMap[appId] = initializeAnalytics( + // Factory will have thrown if options has no appId. + initializationPromisesMap[app.options.appId!] = _initializeAnalytics( app, dynamicConfigPromisesList, measurementIdToAppId, installations, gtagCoreFunction, - dataLayerName + dataLayerName, + options ); - - const analyticsInstance: AnalyticsService = new AnalyticsService(app); - - return analyticsInstance; } diff --git a/packages-exp/analytics-exp/src/index.test.ts b/packages-exp/analytics-exp/src/index.test.ts index 1438d95e31b..01c6857020c 100644 --- a/packages-exp/analytics-exp/src/index.test.ts +++ b/packages-exp/analytics-exp/src/index.test.ts @@ -34,7 +34,8 @@ import { AnalyticsService, getGlobalVars, resetGlobalVars, - factory as analyticsFactory + factory as analyticsFactory, + _initializeAnalyticsForApp } from './factory'; import { _FirebaseInstallationsInternal } from '@firebase/installations-exp'; @@ -89,15 +90,11 @@ describe('FirebaseAnalytics instance tests', () => { it('Throws if no appId in config', () => { const app = getFakeApp({ apiKey: fakeAppParams.apiKey }); - expect(() => analyticsFactory(app, fakeInstallations)).to.throw( - AnalyticsError.NO_APP_ID - ); + expect(() => analyticsFactory(app)).to.throw(AnalyticsError.NO_APP_ID); }); it('Throws if no apiKey or measurementId in config', () => { const app = getFakeApp({ appId: fakeAppParams.appId }); - expect(() => analyticsFactory(app, fakeInstallations)).to.throw( - AnalyticsError.NO_API_KEY - ); + expect(() => analyticsFactory(app)).to.throw(AnalyticsError.NO_API_KEY); }); it('Warns if config has no apiKey but does have a measurementId', async () => { // Since this is a warning and doesn't block the rest of initialization @@ -110,7 +107,8 @@ describe('FirebaseAnalytics instance tests', () => { measurementId: fakeMeasurementId }); stubIdbOpen(); - analyticsFactory(app, fakeInstallations); + analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); // Lets async IDB validation process complete. @@ -128,7 +126,7 @@ describe('FirebaseAnalytics instance tests', () => { it('Throws if creating an instance with already-used appId', () => { const app = getFakeApp(fakeAppParams); resetGlobalVars(false, { [fakeAppParams.appId]: Promise.resolve() }); - expect(() => analyticsFactory(app, fakeInstallations)).to.throw( + expect(() => analyticsFactory(app)).to.throw( AnalyticsError.ALREADY_EXISTS ); }); @@ -149,7 +147,8 @@ describe('FirebaseAnalytics instance tests', () => { window['dataLayer'] = []; stubFetch(200, { measurementId: fakeMeasurementId }); stubIdbOpen(); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); }); @@ -223,7 +222,8 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on initialization if cookies not available', async () => { cookieStub = stub(navigator, 'cookieEnabled').value(false); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); expect(warnStub.args[0][1]).to.include( @@ -234,7 +234,8 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on initialization if in browser extension', async () => { window.chrome = { runtime: { id: 'blah' } }; - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); expect(warnStub.args[0][1]).to.include( @@ -245,7 +246,8 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on logEvent if indexedDB API not available', async () => { const idbStub = stub(window, 'indexedDB').value(undefined); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); logEvent(analyticsInstance, 'add_payment_info', { currency: 'USD' }); @@ -265,7 +267,8 @@ describe('FirebaseAnalytics instance tests', () => { it('Warns on logEvent if indexedDB.open() not allowed', async () => { idbOpenStub.restore(); idbOpenStub = stub(indexedDB, 'open').throws('idb open error test'); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); logEvent(analyticsInstance, 'add_payment_info', { currency: 'USD' }); @@ -303,7 +306,8 @@ describe('FirebaseAnalytics instance tests', () => { }); stubIdbOpen(); stubFetch(200, { measurementId: fakeMeasurementId }); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); }); @@ -349,7 +353,8 @@ describe('FirebaseAnalytics instance tests', () => { fakeInstallations = getFakeInstallations(); stubFetch(200, {}); stubIdbOpen(); - analyticsInstance = analyticsFactory(app, fakeInstallations); + analyticsInstance = analyticsFactory(app); + _initializeAnalyticsForApp(app, fakeInstallations); const { initializationPromisesMap } = getGlobalVars(); // Successfully resolves fake IDB open request. diff --git a/packages-exp/analytics-exp/src/index.ts b/packages-exp/analytics-exp/src/index.ts index 0ec40773f81..ee96166046d 100644 --- a/packages-exp/analytics-exp/src/index.ts +++ b/packages-exp/analytics-exp/src/index.ts @@ -49,11 +49,8 @@ function registerAnalytics(): void { container => { // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app-exp').getImmediate(); - const installations = container - .getProvider('installations-exp-internal') - .getImmediate(); - return factory(app, installations); + return factory(app); }, ComponentType.PUBLIC ) @@ -68,6 +65,8 @@ function registerAnalytics(): void { function internalFactory( container: ComponentContainer ): FirebaseAnalyticsInternal { + //TODO: initialization fetches aren't in factory anymore so it might need to be + // called here. try { const analytics = container.getProvider(ANALYTICS_TYPE).getImmediate(); return { diff --git a/packages-exp/analytics-exp/src/initialize-analytics.test.ts b/packages-exp/analytics-exp/src/initialize-analytics.test.ts index a4bea39eb3c..92745bebbff 100644 --- a/packages-exp/analytics-exp/src/initialize-analytics.test.ts +++ b/packages-exp/analytics-exp/src/initialize-analytics.test.ts @@ -18,7 +18,7 @@ import { expect } from 'chai'; import { SinonStub, stub } from 'sinon'; import '../testing/setup'; -import { initializeAnalytics } from './initialize-analytics'; +import { _initializeAnalytics } from './initialize-analytics'; import { getFakeApp, getFakeInstallations @@ -65,7 +65,7 @@ describe('initializeIds()', () => { }); it('gets FID and measurement ID and calls gtag config with them', async () => { stubFetch(); - await initializeAnalytics( + await _initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -81,7 +81,7 @@ describe('initializeIds()', () => { }); it('puts dynamic fetch promise into dynamic promises list', async () => { stubFetch(); - await initializeAnalytics( + await _initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -95,7 +95,7 @@ describe('initializeIds()', () => { }); it('puts dynamically fetched measurementId into lookup table', async () => { stubFetch(); - await initializeAnalytics( + await _initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -108,7 +108,7 @@ describe('initializeIds()', () => { it('warns on local/fetched measurement ID mismatch', async () => { stubFetch(); const consoleStub = stub(console, 'warn'); - await initializeAnalytics( + await _initializeAnalytics( getFakeApp({ ...fakeAppParams, measurementId: 'old-measurement-id' }), dynamicPromisesList, measurementIdToAppId, diff --git a/packages-exp/analytics-exp/src/initialize-analytics.ts b/packages-exp/analytics-exp/src/initialize-analytics.ts index c9577735b48..6ef0ec54e65 100644 --- a/packages-exp/analytics-exp/src/initialize-analytics.ts +++ b/packages-exp/analytics-exp/src/initialize-analytics.ts @@ -27,6 +27,7 @@ import { } from '@firebase/util'; import { ERROR_FACTORY, AnalyticsError } from './errors'; import { findGtagScriptOnPage, insertScriptTag } from './helpers'; +import { AnalyticsOptions } from './public-types'; async function validateIndexedDB(): Promise { if (!isIndexedDBAvailable()) { @@ -64,7 +65,7 @@ async function validateIndexedDB(): Promise { * * @returns Measurement ID. */ -export async function initializeAnalytics( +export async function _initializeAnalytics( app: FirebaseApp, dynamicConfigPromisesList: Array< Promise @@ -72,7 +73,8 @@ export async function initializeAnalytics( measurementIdToAppId: { [key: string]: string }, installations: _FirebaseInstallationsInternal, gtagCore: Gtag, - dataLayerName: string + dataLayerName: string, + options?: AnalyticsOptions ): Promise { const dynamicConfigPromise = fetchDynamicConfigWithRetry(app); // Once fetched, map measurementIds to appId, for ease of lookup in wrapped gtag function. @@ -122,11 +124,13 @@ export async function initializeAnalytics( // eslint-disable-next-line @typescript-eslint/no-explicit-any (gtagCore as any)('js', new Date()); - const configProperties: { [key: string]: string | boolean } = { - // guard against developers accidentally setting properties with prefix `firebase_` - [ORIGIN_KEY]: 'firebase', - update: true - }; + // User config added first. We don't want users to accidentally overwrite + // base Firebase config properties. + const configProperties: Record = options?.config ?? {}; + + // guard against developers accidentally setting properties with prefix `firebase_` + configProperties[ORIGIN_KEY] = 'firebase'; + configProperties.update = true; if (fid != null) { configProperties[GA_FID_KEY] = fid; diff --git a/packages-exp/analytics-exp/src/public-types.ts b/packages-exp/analytics-exp/src/public-types.ts index fe7213af164..70e941d1811 100644 --- a/packages-exp/analytics-exp/src/public-types.ts +++ b/packages-exp/analytics-exp/src/public-types.ts @@ -17,6 +17,35 @@ import { FirebaseApp } from '@firebase/app-exp'; +/** + * A set of common Analytics config settings recognized by + * gtag. + */ +export interface GtagConfigParams { + 'send_page_view'?: boolean; + 'page_title'?: string; + 'page_path'?: string; + 'page_location'?: string; + 'cookie_domain'?: string; + 'cookie_expires'?: number; + 'cookie_prefix'?: string; + 'cookie_update'?: boolean; + 'cookie_flags'?: string; + 'allow_google_signals?': boolean; + 'allow_ad_personalization_signals'?: boolean; + 'link_attribution'?: boolean; + 'anonymize_ip'?: boolean; + 'custom_map'?: { [key: string]: unknown }; + [key: string]: unknown; +} + +/** + * Analytics initialization options. + */ +export interface AnalyticsOptions { + config: GtagConfigParams | EventParams; +} + /** * Additional options that can be passed to Firebase Analytics method * calls such as `logEvent`, `setCurrentScreen`, etc. @@ -204,5 +233,6 @@ export interface EventParams { page_title?: string; page_location?: string; page_path?: string; + [key: string]: unknown; } /* eslint-enable camelcase */ From 0e6d2898c3046fb5e90ef86dad46276a59850242 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 2 Mar 2021 10:33:16 -0800 Subject: [PATCH 2/7] Fix tests and type comments --- common/api-review/analytics-exp.api.md | 47 ++++++++++ packages-exp/analytics-exp/src/api.ts | 5 +- packages-exp/analytics-exp/src/errors.ts | 5 + packages-exp/analytics-exp/src/factory.ts | 2 +- .../analytics-exp/src/public-types.ts | 5 +- .../testing/integration-tests/integration.ts | 91 +++++++++++++------ 6 files changed, 121 insertions(+), 34 deletions(-) diff --git a/common/api-review/analytics-exp.api.md b/common/api-review/analytics-exp.api.md index cbdf8be9b83..6dacc876dd6 100644 --- a/common/api-review/analytics-exp.api.md +++ b/common/api-review/analytics-exp.api.md @@ -16,6 +16,12 @@ export interface AnalyticsCallOptions { global: boolean; } +// @public +export interface AnalyticsOptions { + // (undocumented) + config: GtagConfigParams | EventParams; +} + // @public export interface ControlParams { // (undocumented) @@ -45,6 +51,8 @@ export type EventNameString = 'add_payment_info' | 'add_shipping_info' | 'add_to // @public export interface EventParams { + // (undocumented) + [key: string]: unknown; // (undocumented) affiliation?: string; // (undocumented) @@ -110,6 +118,45 @@ export interface EventParams { // @public export function getAnalytics(app?: FirebaseApp): Analytics; +// @public +export interface GtagConfigParams { + // (undocumented) + 'allow_google_signals?': boolean; + // (undocumented) + [key: string]: unknown; + // (undocumented) + 'allow_ad_personalization_signals'?: boolean; + // (undocumented) + 'anonymize_ip'?: boolean; + // (undocumented) + 'cookie_domain'?: string; + // (undocumented) + 'cookie_expires'?: number; + // (undocumented) + 'cookie_flags'?: string; + // (undocumented) + 'cookie_prefix'?: string; + // (undocumented) + 'cookie_update'?: boolean; + // (undocumented) + 'custom_map'?: { + [key: string]: unknown; + }; + // (undocumented) + 'link_attribution'?: boolean; + // (undocumented) + 'page_location'?: string; + // (undocumented) + 'page_path'?: string; + // (undocumented) + 'page_title'?: string; + // (undocumented) + 'send_page_view'?: boolean; +} + +// @public +export function initializeAnalytics(app: FirebaseApp, options?: AnalyticsOptions): Analytics; + // @public export function isSupported(): Promise; diff --git a/packages-exp/analytics-exp/src/api.ts b/packages-exp/analytics-exp/src/api.ts index e44d8f71a89..f539a33c100 100644 --- a/packages-exp/analytics-exp/src/api.ts +++ b/packages-exp/analytics-exp/src/api.ts @@ -49,6 +49,7 @@ import { setUserProperties as internalSetUserProperties, setAnalyticsCollectionEnabled as internalSetAnalyticsCollectionEnabled } from './functions'; +import { ERROR_FACTORY, AnalyticsError } from './errors'; export { settings } from './factory'; @@ -97,9 +98,7 @@ export function initializeAnalytics( ANALYTICS_TYPE ); if (analyticsProvider.isInitialized()) { - return analyticsProvider.getImmediate(); - // TODO: Throw an analytics specific error. - // _fail(analyticsInstance, AuthErrorCode.ALREADY_INITIALIZED); + throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED); } const analyticsInstance = analyticsProvider.getImmediate(); // do init settings stuff here diff --git a/packages-exp/analytics-exp/src/errors.ts b/packages-exp/analytics-exp/src/errors.ts index 6381eac4871..b500f88d013 100644 --- a/packages-exp/analytics-exp/src/errors.ts +++ b/packages-exp/analytics-exp/src/errors.ts @@ -20,6 +20,7 @@ import { ErrorFactory, ErrorMap } from '@firebase/util'; export const enum AnalyticsError { ALREADY_EXISTS = 'already-exists', ALREADY_INITIALIZED = 'already-initialized', + ALREADY_INITIALIZED_SETTINGS = 'already-initialized-settings', INTEROP_COMPONENT_REG_FAILED = 'interop-component-reg-failed', INVALID_ANALYTICS_CONTEXT = 'invalid-analytics-context', INDEXEDDB_UNAVAILABLE = 'indexeddb-unavailable', @@ -35,6 +36,10 @@ const ERRORS: ErrorMap = { ' already exists. ' + 'Only one Firebase Analytics instance can be created for each appId.', [AnalyticsError.ALREADY_INITIALIZED]: + 'Firebase Analytics has already been initialized. ' + + 'initializeAnalytics() must only be called once. getAnalytics() can be used ' + + 'to get a reference to the already-intialized instance.', + [AnalyticsError.ALREADY_INITIALIZED_SETTINGS]: 'Firebase Analytics has already been initialized.' + 'settings() must be called before initializing any Analytics instance' + 'or it will have no effect.', diff --git a/packages-exp/analytics-exp/src/factory.ts b/packages-exp/analytics-exp/src/factory.ts index cb47074af16..c6dc58af8d1 100644 --- a/packages-exp/analytics-exp/src/factory.ts +++ b/packages-exp/analytics-exp/src/factory.ts @@ -136,7 +136,7 @@ export function getGlobalVars(): { */ export function settings(options: SettingsOptions): void { if (globalInitDone) { - throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED); + throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED_SETTINGS); } if (options.dataLayerName) { dataLayerName = options.dataLayerName; diff --git a/packages-exp/analytics-exp/src/public-types.ts b/packages-exp/analytics-exp/src/public-types.ts index 70e941d1811..1cb665850c1 100644 --- a/packages-exp/analytics-exp/src/public-types.ts +++ b/packages-exp/analytics-exp/src/public-types.ts @@ -20,6 +20,7 @@ import { FirebaseApp } from '@firebase/app-exp'; /** * A set of common Analytics config settings recognized by * gtag. + * @public */ export interface GtagConfigParams { 'send_page_view'?: boolean; @@ -41,6 +42,7 @@ export interface GtagConfigParams { /** * Analytics initialization options. + * @public */ export interface AnalyticsOptions { config: GtagConfigParams | EventParams; @@ -60,8 +62,7 @@ export interface AnalyticsCallOptions { } /** - * The Firebase Analytics service interface. - * + * An instance of Firebase Analytics. * @public */ export interface Analytics { diff --git a/packages-exp/analytics-exp/testing/integration-tests/integration.ts b/packages-exp/analytics-exp/testing/integration-tests/integration.ts index 9ea64a1b65a..c063491995f 100644 --- a/packages-exp/analytics-exp/testing/integration-tests/integration.ts +++ b/packages-exp/analytics-exp/testing/integration-tests/integration.ts @@ -17,10 +17,11 @@ import { initializeApp, deleteApp, FirebaseApp } from '@firebase/app-exp'; import '@firebase/installations-exp'; -import { getAnalytics, logEvent } from '../../src/index'; +import { getAnalytics, initializeAnalytics, logEvent } from '../../src/index'; import '../setup'; import { expect } from 'chai'; import { stub } from 'sinon'; +import { AnalyticsError } from '../../src/errors'; let config: Record; try { @@ -36,36 +37,70 @@ const RETRY_INTERVAL = 1000; describe('FirebaseAnalytics Integration Smoke Tests', () => { let app: FirebaseApp; - afterEach(() => deleteApp(app)); - it('logEvent() sends correct network request.', async () => { - app = initializeApp(config); - logEvent(getAnalytics(app), 'login', { method: 'email' }); - async function checkForEventCalls(): Promise { - await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); - const resources = performance.getEntriesByType('resource'); - const callsWithEvent = resources.filter( - resource => - resource.name.includes('google-analytics.com') && - resource.name.includes('en=login') - ); - if (callsWithEvent.length === 0) { - return checkForEventCalls(); - } else { - return callsWithEvent.length; + describe('Using getAnalytics()', () => { + afterEach(() => deleteApp(app)); + it('logEvent() sends correct network request.', async () => { + app = initializeApp(config); + logEvent(getAnalytics(app), 'login', { method: 'email' }); + async function checkForEventCalls(): Promise { + await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); + const resources = performance.getEntriesByType('resource'); + const callsWithEvent = resources.filter( + resource => + resource.name.includes('google-analytics.com') && + resource.name.includes('en=login') + ); + if (callsWithEvent.length === 0) { + return checkForEventCalls(); + } else { + return callsWithEvent.length; + } } - } - const eventCallCount = await checkForEventCalls(); - expect(eventCallCount).to.equal(1); + const eventCallCount = await checkForEventCalls(); + expect(eventCallCount).to.equal(1); + }); + it("Warns if measurement ID doesn't match.", done => { + const warnStub = stub(console, 'warn').callsFake(() => { + expect(warnStub.args[0][1]).to.include('does not match'); + done(); + }); + app = initializeApp({ + ...config, + measurementId: 'wrong-id' + }); + getAnalytics(app); + }); }); - it("Warns if measurement ID doesn't match.", done => { - const warnStub = stub(console, 'warn').callsFake(() => { - expect(warnStub.args[0][1]).to.include('does not match'); - done(); + describe('Using initializeAnalytics()', () => { + it('logEvent() sends correct network request.', async () => { + app = initializeApp(config); + logEvent(initializeAnalytics(app), 'login', { method: 'email' }); + async function checkForEventCalls(): Promise { + await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); + const resources = performance.getEntriesByType('resource'); + const callsWithEvent = resources.filter( + resource => + resource.name.includes('google-analytics.com') && + resource.name.includes('en=login') + ); + if (callsWithEvent.length === 0) { + return checkForEventCalls(); + } else { + return callsWithEvent.length; + } + } + const eventCallCount = await checkForEventCalls(); + expect(eventCallCount).to.equal(1); + }); + it('getAnalytics() does not throw if called after initializeAnalytics().', async () => { + const analyticsInstance = getAnalytics(app); + expect(analyticsInstance.app).to.equal(app); }); - app = initializeApp({ - ...config, - measurementId: 'wrong-id' + it('initializeAnalytics() throws if called more than once.', async () => { + expect(() => initializeAnalytics(app)).to.throw( + AnalyticsError.ALREADY_INITIALIZED + ); + await deleteApp(app); }); - getAnalytics(app); }); }); From 63c2b71faa110f1636e7b152fe3ff1654d8c99ca Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 2 Mar 2021 14:03:49 -0800 Subject: [PATCH 3/7] Add some documentation --- .../analytics-exp/src/public-types.ts | 66 ++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/packages-exp/analytics-exp/src/public-types.ts b/packages-exp/analytics-exp/src/public-types.ts index 1cb665850c1..0ae23179334 100644 --- a/packages-exp/analytics-exp/src/public-types.ts +++ b/packages-exp/analytics-exp/src/public-types.ts @@ -23,19 +23,77 @@ import { FirebaseApp } from '@firebase/app-exp'; * @public */ export interface GtagConfigParams { + /** + * Whether or not a page view should be sent. + * If set to true (default), a page view is automatically sent upon initialization + * of analytics. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/pages + */ 'send_page_view'?: boolean; + /** + * The title of the page. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/pages + */ 'page_title'?: string; + /** + * The path to the page. If overridden, this value must start with a / character. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/pages + */ 'page_path'?: string; + /** + * The URL of the page. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/pages + */ 'page_location'?: string; + /** + * Defaults to `auto`. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/cookies-user-id + */ 'cookie_domain'?: string; + /** + * Defaults to 63072000 (two years, in seconds). + * See https://developers.google.com/analytics/devguides/collection/gtagjs/cookies-user-id + */ 'cookie_expires'?: number; + /** + * Defaults to `_ga`. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/cookies-user-id + */ 'cookie_prefix'?: string; + /** + * If set to true, will update cookies on each page load. + * Defaults to true. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/cookies-user-id + */ 'cookie_update'?: boolean; + /** + * Appends additional flags to the cookie when set. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/cookies-user-id + */ 'cookie_flags'?: string; + /** + * If set to false, disables all advertising features with gtag.js. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/display-features + */ 'allow_google_signals?': boolean; + /** + * If set to false, disables all advertising personalization with gtag.js. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/display-features + */ 'allow_ad_personalization_signals'?: boolean; + /** + * See https://developers.google.com/analytics/devguides/collection/gtagjs/enhanced-link-attribution + */ 'link_attribution'?: boolean; + /** + * If set to true, anonymizes IP addresses for all events. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/ip-anonymization + */ 'anonymize_ip'?: boolean; + /** + * Custom dimensions and metrics. + * See https://developers.google.com/analytics/devguides/collection/gtagjs/custom-dims-mets + */ 'custom_map'?: { [key: string]: unknown }; [key: string]: unknown; } @@ -45,6 +103,9 @@ export interface GtagConfigParams { * @public */ export interface AnalyticsOptions { + /** + * Params to be passed in the initial gtag config call during analytics initialization. + */ config: GtagConfigParams | EventParams; } @@ -91,6 +152,7 @@ export interface SettingsOptions { export interface CustomParams { [key: string]: unknown; } + /** * Type for standard gtag.js event names. `logEvent` also accepts any * custom string and interprets it as a custom event name. @@ -126,14 +188,14 @@ export type EventNameString = | 'view_search_results'; /** - * Currency field used by some Analytics events. + * Standard analytics currency type. * @public */ export type Currency = string | number; /* eslint-disable camelcase */ /** - * Item field used by some Analytics events. + * Standard analytics `Item` type. * @public */ export interface Item { From 83092d4c4b95e261a8fac4d4a30be754ae3ddaab Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Tue, 2 Mar 2021 14:04:47 -0800 Subject: [PATCH 4/7] Update API report --- common/api-review/analytics-exp.api.md | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/common/api-review/analytics-exp.api.md b/common/api-review/analytics-exp.api.md index 6dacc876dd6..8ae65a5526b 100644 --- a/common/api-review/analytics-exp.api.md +++ b/common/api-review/analytics-exp.api.md @@ -18,7 +18,6 @@ export interface AnalyticsCallOptions { // @public export interface AnalyticsOptions { - // (undocumented) config: GtagConfigParams | EventParams; } @@ -120,37 +119,23 @@ export function getAnalytics(app?: FirebaseApp): Analytics; // @public export interface GtagConfigParams { - // (undocumented) 'allow_google_signals?': boolean; // (undocumented) [key: string]: unknown; - // (undocumented) 'allow_ad_personalization_signals'?: boolean; - // (undocumented) 'anonymize_ip'?: boolean; - // (undocumented) 'cookie_domain'?: string; - // (undocumented) 'cookie_expires'?: number; - // (undocumented) 'cookie_flags'?: string; - // (undocumented) 'cookie_prefix'?: string; - // (undocumented) 'cookie_update'?: boolean; - // (undocumented) 'custom_map'?: { [key: string]: unknown; }; - // (undocumented) 'link_attribution'?: boolean; - // (undocumented) 'page_location'?: string; - // (undocumented) 'page_path'?: string; - // (undocumented) 'page_title'?: string; - // (undocumented) 'send_page_view'?: boolean; } From 0e26c1573976a6ddbe2f15970393edc5b2a5f289 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 17 Mar 2021 10:32:08 -0700 Subject: [PATCH 5/7] Implement using provider.initialize() --- common/api-review/analytics-exp.api.md | 2 +- packages-exp/analytics-exp/src/api.ts | 11 +----- packages-exp/analytics-exp/src/factory.ts | 30 ++++++--------- packages-exp/analytics-exp/src/index.test.ts | 37 ++++++++----------- packages-exp/analytics-exp/src/index.ts | 12 +++--- .../src/initialize-analytics.test.ts | 10 ++--- .../analytics-exp/src/initialize-analytics.ts | 3 +- .../analytics-exp/src/public-types.ts | 2 +- 8 files changed, 45 insertions(+), 62 deletions(-) diff --git a/common/api-review/analytics-exp.api.md b/common/api-review/analytics-exp.api.md index 8ae65a5526b..35172fa0f8f 100644 --- a/common/api-review/analytics-exp.api.md +++ b/common/api-review/analytics-exp.api.md @@ -18,7 +18,7 @@ export interface AnalyticsCallOptions { // @public export interface AnalyticsOptions { - config: GtagConfigParams | EventParams; + config?: GtagConfigParams | EventParams; } // @public diff --git a/packages-exp/analytics-exp/src/api.ts b/packages-exp/analytics-exp/src/api.ts index f539a33c100..c0ff7f337fc 100644 --- a/packages-exp/analytics-exp/src/api.ts +++ b/packages-exp/analytics-exp/src/api.ts @@ -38,7 +38,6 @@ import { ANALYTICS_TYPE } from './constants'; import { AnalyticsService, initializationPromisesMap, - _initializeAnalyticsForApp, wrappedGtagFunction } from './factory'; import { logger } from './logger'; @@ -90,7 +89,7 @@ export function getAnalytics(app: FirebaseApp = getApp()): Analytics { */ export function initializeAnalytics( app: FirebaseApp, - options?: AnalyticsOptions + options: AnalyticsOptions = {} ): Analytics { // Dependencies const analyticsProvider: Provider<'analytics-exp'> = _getProvider( @@ -100,13 +99,7 @@ export function initializeAnalytics( if (analyticsProvider.isInitialized()) { throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED); } - const analyticsInstance = analyticsProvider.getImmediate(); - // do init settings stuff here - const installations = _getProvider( - app, - 'installations-exp-internal' - ).getImmediate(); - _initializeAnalyticsForApp(app, installations, options); + const analyticsInstance = analyticsProvider.initialize({ options }); return analyticsInstance; } diff --git a/packages-exp/analytics-exp/src/factory.ts b/packages-exp/analytics-exp/src/factory.ts index c6dc58af8d1..e43535a3558 100644 --- a/packages-exp/analytics-exp/src/factory.ts +++ b/packages-exp/analytics-exp/src/factory.ts @@ -21,7 +21,7 @@ import { getOrCreateDataLayer, wrapOrCreateGtag } from './helpers'; import { AnalyticsError, ERROR_FACTORY } from './errors'; import { _FirebaseInstallationsInternal } from '@firebase/installations-exp'; import { areCookiesEnabled, isBrowserExtension } from '@firebase/util'; -import { _initializeAnalytics } from './initialize-analytics'; +import { initializeAnalytics } from './initialize-analytics'; import { logger } from './logger'; import { FirebaseApp, _FirebaseService } from '@firebase/app-exp'; @@ -136,7 +136,7 @@ export function getGlobalVars(): { */ export function settings(options: SettingsOptions): void { if (globalInitDone) { - throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED_SETTINGS); + throw ERROR_FACTORY.create(AnalyticsError.ALREADY_INITIALIZED); } if (options.dataLayerName) { dataLayerName = options.dataLayerName; @@ -174,7 +174,11 @@ function warnOnBrowserContextMismatch(): void { * Analytics instance factory. * @internal */ -export function factory(app: FirebaseApp): AnalyticsService { +export function factory( + app: FirebaseApp, + installations: _FirebaseInstallationsInternal, + options?: AnalyticsOptions +): AnalyticsService { warnOnBrowserContextMismatch(); const appId = app.options.appId; if (!appId) { @@ -215,23 +219,9 @@ export function factory(app: FirebaseApp): AnalyticsService { globalInitDone = true; } - - const analyticsInstance: AnalyticsService = new AnalyticsService(app); - return analyticsInstance; -} - -/** - * Starts async initializations for this app. - */ -export function _initializeAnalyticsForApp( - app: FirebaseApp, - installations: _FirebaseInstallationsInternal, - options?: AnalyticsOptions -): void { // Async but non-blocking. // This map reflects the completion state of all promises for each appId. - // Factory will have thrown if options has no appId. - initializationPromisesMap[app.options.appId!] = _initializeAnalytics( + initializationPromisesMap[appId] = initializeAnalytics( app, dynamicConfigPromisesList, measurementIdToAppId, @@ -240,4 +230,8 @@ export function _initializeAnalyticsForApp( dataLayerName, options ); + + const analyticsInstance: AnalyticsService = new AnalyticsService(app); + + return analyticsInstance; } diff --git a/packages-exp/analytics-exp/src/index.test.ts b/packages-exp/analytics-exp/src/index.test.ts index 01c6857020c..1438d95e31b 100644 --- a/packages-exp/analytics-exp/src/index.test.ts +++ b/packages-exp/analytics-exp/src/index.test.ts @@ -34,8 +34,7 @@ import { AnalyticsService, getGlobalVars, resetGlobalVars, - factory as analyticsFactory, - _initializeAnalyticsForApp + factory as analyticsFactory } from './factory'; import { _FirebaseInstallationsInternal } from '@firebase/installations-exp'; @@ -90,11 +89,15 @@ describe('FirebaseAnalytics instance tests', () => { it('Throws if no appId in config', () => { const app = getFakeApp({ apiKey: fakeAppParams.apiKey }); - expect(() => analyticsFactory(app)).to.throw(AnalyticsError.NO_APP_ID); + expect(() => analyticsFactory(app, fakeInstallations)).to.throw( + AnalyticsError.NO_APP_ID + ); }); it('Throws if no apiKey or measurementId in config', () => { const app = getFakeApp({ appId: fakeAppParams.appId }); - expect(() => analyticsFactory(app)).to.throw(AnalyticsError.NO_API_KEY); + expect(() => analyticsFactory(app, fakeInstallations)).to.throw( + AnalyticsError.NO_API_KEY + ); }); it('Warns if config has no apiKey but does have a measurementId', async () => { // Since this is a warning and doesn't block the rest of initialization @@ -107,8 +110,7 @@ describe('FirebaseAnalytics instance tests', () => { measurementId: fakeMeasurementId }); stubIdbOpen(); - analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsFactory(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); // Lets async IDB validation process complete. @@ -126,7 +128,7 @@ describe('FirebaseAnalytics instance tests', () => { it('Throws if creating an instance with already-used appId', () => { const app = getFakeApp(fakeAppParams); resetGlobalVars(false, { [fakeAppParams.appId]: Promise.resolve() }); - expect(() => analyticsFactory(app)).to.throw( + expect(() => analyticsFactory(app, fakeInstallations)).to.throw( AnalyticsError.ALREADY_EXISTS ); }); @@ -147,8 +149,7 @@ describe('FirebaseAnalytics instance tests', () => { window['dataLayer'] = []; stubFetch(200, { measurementId: fakeMeasurementId }); stubIdbOpen(); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); }); @@ -222,8 +223,7 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on initialization if cookies not available', async () => { cookieStub = stub(navigator, 'cookieEnabled').value(false); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); expect(warnStub.args[0][1]).to.include( @@ -234,8 +234,7 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on initialization if in browser extension', async () => { window.chrome = { runtime: { id: 'blah' } }; - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); expect(warnStub.args[0][1]).to.include( @@ -246,8 +245,7 @@ describe('FirebaseAnalytics instance tests', () => { }); it('Warns on logEvent if indexedDB API not available', async () => { const idbStub = stub(window, 'indexedDB').value(undefined); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); logEvent(analyticsInstance, 'add_payment_info', { currency: 'USD' }); @@ -267,8 +265,7 @@ describe('FirebaseAnalytics instance tests', () => { it('Warns on logEvent if indexedDB.open() not allowed', async () => { idbOpenStub.restore(); idbOpenStub = stub(indexedDB, 'open').throws('idb open error test'); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); logEvent(analyticsInstance, 'add_payment_info', { currency: 'USD' }); @@ -306,8 +303,7 @@ describe('FirebaseAnalytics instance tests', () => { }); stubIdbOpen(); stubFetch(200, { measurementId: fakeMeasurementId }); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); // Successfully resolves fake IDB open request. fakeRequest.onsuccess(); }); @@ -353,8 +349,7 @@ describe('FirebaseAnalytics instance tests', () => { fakeInstallations = getFakeInstallations(); stubFetch(200, {}); stubIdbOpen(); - analyticsInstance = analyticsFactory(app); - _initializeAnalyticsForApp(app, fakeInstallations); + analyticsInstance = analyticsFactory(app, fakeInstallations); const { initializationPromisesMap } = getGlobalVars(); // Successfully resolves fake IDB open request. diff --git a/packages-exp/analytics-exp/src/index.ts b/packages-exp/analytics-exp/src/index.ts index ee96166046d..d4050799802 100644 --- a/packages-exp/analytics-exp/src/index.ts +++ b/packages-exp/analytics-exp/src/index.ts @@ -28,7 +28,8 @@ import { ANALYTICS_TYPE } from './constants'; import { Component, ComponentType, - ComponentContainer + ComponentContainer, + InstanceFactoryOptions } from '@firebase/component'; import { ERROR_FACTORY, AnalyticsError } from './errors'; import { logEvent } from './api'; @@ -46,11 +47,14 @@ function registerAnalytics(): void { _registerComponent( new Component( ANALYTICS_TYPE, - container => { + (container, { options: analyticsOptions }: InstanceFactoryOptions) => { // getImmediate for FirebaseApp will always succeed const app = container.getProvider('app-exp').getImmediate(); + const installations = container + .getProvider('installations-exp-internal') + .getImmediate(); - return factory(app); + return factory(app, installations, analyticsOptions); }, ComponentType.PUBLIC ) @@ -65,8 +69,6 @@ function registerAnalytics(): void { function internalFactory( container: ComponentContainer ): FirebaseAnalyticsInternal { - //TODO: initialization fetches aren't in factory anymore so it might need to be - // called here. try { const analytics = container.getProvider(ANALYTICS_TYPE).getImmediate(); return { diff --git a/packages-exp/analytics-exp/src/initialize-analytics.test.ts b/packages-exp/analytics-exp/src/initialize-analytics.test.ts index 92745bebbff..a4bea39eb3c 100644 --- a/packages-exp/analytics-exp/src/initialize-analytics.test.ts +++ b/packages-exp/analytics-exp/src/initialize-analytics.test.ts @@ -18,7 +18,7 @@ import { expect } from 'chai'; import { SinonStub, stub } from 'sinon'; import '../testing/setup'; -import { _initializeAnalytics } from './initialize-analytics'; +import { initializeAnalytics } from './initialize-analytics'; import { getFakeApp, getFakeInstallations @@ -65,7 +65,7 @@ describe('initializeIds()', () => { }); it('gets FID and measurement ID and calls gtag config with them', async () => { stubFetch(); - await _initializeAnalytics( + await initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -81,7 +81,7 @@ describe('initializeIds()', () => { }); it('puts dynamic fetch promise into dynamic promises list', async () => { stubFetch(); - await _initializeAnalytics( + await initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -95,7 +95,7 @@ describe('initializeIds()', () => { }); it('puts dynamically fetched measurementId into lookup table', async () => { stubFetch(); - await _initializeAnalytics( + await initializeAnalytics( app, dynamicPromisesList, measurementIdToAppId, @@ -108,7 +108,7 @@ describe('initializeIds()', () => { it('warns on local/fetched measurement ID mismatch', async () => { stubFetch(); const consoleStub = stub(console, 'warn'); - await _initializeAnalytics( + await initializeAnalytics( getFakeApp({ ...fakeAppParams, measurementId: 'old-measurement-id' }), dynamicPromisesList, measurementIdToAppId, diff --git a/packages-exp/analytics-exp/src/initialize-analytics.ts b/packages-exp/analytics-exp/src/initialize-analytics.ts index 6ef0ec54e65..3eab2f3da9b 100644 --- a/packages-exp/analytics-exp/src/initialize-analytics.ts +++ b/packages-exp/analytics-exp/src/initialize-analytics.ts @@ -65,7 +65,7 @@ async function validateIndexedDB(): Promise { * * @returns Measurement ID. */ -export async function _initializeAnalytics( +export async function initializeAnalytics( app: FirebaseApp, dynamicConfigPromisesList: Array< Promise @@ -123,7 +123,6 @@ export async function _initializeAnalytics( // We keep it together with other initialization logic for better code structure. // eslint-disable-next-line @typescript-eslint/no-explicit-any (gtagCore as any)('js', new Date()); - // User config added first. We don't want users to accidentally overwrite // base Firebase config properties. const configProperties: Record = options?.config ?? {}; diff --git a/packages-exp/analytics-exp/src/public-types.ts b/packages-exp/analytics-exp/src/public-types.ts index 0ae23179334..c97835c21c0 100644 --- a/packages-exp/analytics-exp/src/public-types.ts +++ b/packages-exp/analytics-exp/src/public-types.ts @@ -106,7 +106,7 @@ export interface AnalyticsOptions { /** * Params to be passed in the initial gtag config call during analytics initialization. */ - config: GtagConfigParams | EventParams; + config?: GtagConfigParams | EventParams; } /** From d7846c1a2cd0b32f34ded3955369ba88d88ab5f7 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Wed, 17 Mar 2021 10:48:26 -0700 Subject: [PATCH 6/7] Add a test --- .../src/initialize-analytics.test.ts | 20 ++++++++++++++++++- .../testing/integration-tests/integration.ts | 9 +++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages-exp/analytics-exp/src/initialize-analytics.test.ts b/packages-exp/analytics-exp/src/initialize-analytics.test.ts index a4bea39eb3c..93d2bc8d4b9 100644 --- a/packages-exp/analytics-exp/src/initialize-analytics.test.ts +++ b/packages-exp/analytics-exp/src/initialize-analytics.test.ts @@ -48,7 +48,7 @@ function stubFetch(): void { fetchStub.returns(Promise.resolve(mockResponse)); } -describe('initializeIds()', () => { +describe('initializeAnalytics()', () => { const gtagStub: SinonStub = stub(); const dynamicPromisesList: Array> = []; const measurementIdToAppId: { [key: string]: string } = {}; @@ -79,6 +79,24 @@ describe('initializeIds()', () => { update: true }); }); + it('calls gtag config with options if provided', async () => { + stubFetch(); + await initializeAnalytics( + app, + dynamicPromisesList, + measurementIdToAppId, + fakeInstallations, + gtagStub, + 'dataLayer', + { config: { 'send_page_view': false } } + ); + expect(gtagStub).to.be.calledWith(GtagCommand.CONFIG, fakeMeasurementId, { + 'firebase_id': fakeFid, + 'origin': 'firebase', + update: true, + 'send_page_view': false + }); + }); it('puts dynamic fetch promise into dynamic promises list', async () => { stubFetch(); await initializeAnalytics( diff --git a/packages-exp/analytics-exp/testing/integration-tests/integration.ts b/packages-exp/analytics-exp/testing/integration-tests/integration.ts index c063491995f..20665ae84a8 100644 --- a/packages-exp/analytics-exp/testing/integration-tests/integration.ts +++ b/packages-exp/analytics-exp/testing/integration-tests/integration.ts @@ -75,7 +75,7 @@ describe('FirebaseAnalytics Integration Smoke Tests', () => { it('logEvent() sends correct network request.', async () => { app = initializeApp(config); logEvent(initializeAnalytics(app), 'login', { method: 'email' }); - async function checkForEventCalls(): Promise { + async function checkForEventCalls(): Promise { await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); const resources = performance.getEntriesByType('resource'); const callsWithEvent = resources.filter( @@ -86,11 +86,12 @@ describe('FirebaseAnalytics Integration Smoke Tests', () => { if (callsWithEvent.length === 0) { return checkForEventCalls(); } else { - return callsWithEvent.length; + return callsWithEvent; } } - const eventCallCount = await checkForEventCalls(); - expect(eventCallCount).to.equal(1); + const eventCalls = await checkForEventCalls(); + expect(eventCalls.length).to.equal(1); + expect(eventCalls[0].name).to.include('method=email'); }); it('getAnalytics() does not throw if called after initializeAnalytics().', async () => { const analyticsInstance = getAnalytics(app); From aa563ba6e78e8ac7ae115aa01a7b31aaee426e75 Mon Sep 17 00:00:00 2001 From: Christina Holland Date: Thu, 20 May 2021 15:09:51 -0700 Subject: [PATCH 7/7] Fix test --- .../analytics-exp/src/public-types.ts | 2 +- .../testing/integration-tests/integration.ts | 55 ++++++++----------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/packages-exp/analytics-exp/src/public-types.ts b/packages-exp/analytics-exp/src/public-types.ts index c97835c21c0..13076472589 100644 --- a/packages-exp/analytics-exp/src/public-types.ts +++ b/packages-exp/analytics-exp/src/public-types.ts @@ -128,7 +128,7 @@ export interface AnalyticsCallOptions { */ export interface Analytics { /** - * The FirebaseApp this Functions instance is associated with. + * The FirebaseApp this Analytics instance is associated with. */ app: FirebaseApp; } diff --git a/packages-exp/analytics-exp/testing/integration-tests/integration.ts b/packages-exp/analytics-exp/testing/integration-tests/integration.ts index 20665ae84a8..e95a709b919 100644 --- a/packages-exp/analytics-exp/testing/integration-tests/integration.ts +++ b/packages-exp/analytics-exp/testing/integration-tests/integration.ts @@ -34,6 +34,26 @@ try { } const RETRY_INTERVAL = 1000; +const TIMEOUT_MILLIS = 20000; + +async function checkForEventCalls(retryCount = 0): Promise { + if (retryCount > TIMEOUT_MILLIS / RETRY_INTERVAL) { + return Promise.resolve([]); + } + await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); + const resources = performance.getEntriesByType('resource'); + performance.clearResourceTimings(); + const callsWithEvent = resources.filter( + resource => + resource.name.includes('google-analytics.com') && + resource.name.includes('en=login') + ); + if (callsWithEvent.length === 0) { + return checkForEventCalls(retryCount + 1); + } else { + return callsWithEvent; + } +} describe('FirebaseAnalytics Integration Smoke Tests', () => { let app: FirebaseApp; @@ -41,23 +61,10 @@ describe('FirebaseAnalytics Integration Smoke Tests', () => { afterEach(() => deleteApp(app)); it('logEvent() sends correct network request.', async () => { app = initializeApp(config); - logEvent(getAnalytics(app), 'login', { method: 'email' }); - async function checkForEventCalls(): Promise { - await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); - const resources = performance.getEntriesByType('resource'); - const callsWithEvent = resources.filter( - resource => - resource.name.includes('google-analytics.com') && - resource.name.includes('en=login') - ); - if (callsWithEvent.length === 0) { - return checkForEventCalls(); - } else { - return callsWithEvent.length; - } - } - const eventCallCount = await checkForEventCalls(); - expect(eventCallCount).to.equal(1); + logEvent(getAnalytics(app), 'login', { method: 'phone' }); + const eventCalls = await checkForEventCalls(); + expect(eventCalls.length).to.equal(1); + expect(eventCalls[0].name).to.include('method=phone'); }); it("Warns if measurement ID doesn't match.", done => { const warnStub = stub(console, 'warn').callsFake(() => { @@ -75,20 +82,6 @@ describe('FirebaseAnalytics Integration Smoke Tests', () => { it('logEvent() sends correct network request.', async () => { app = initializeApp(config); logEvent(initializeAnalytics(app), 'login', { method: 'email' }); - async function checkForEventCalls(): Promise { - await new Promise(resolve => setTimeout(resolve, RETRY_INTERVAL)); - const resources = performance.getEntriesByType('resource'); - const callsWithEvent = resources.filter( - resource => - resource.name.includes('google-analytics.com') && - resource.name.includes('en=login') - ); - if (callsWithEvent.length === 0) { - return checkForEventCalls(); - } else { - return callsWithEvent; - } - } const eventCalls = await checkForEventCalls(); expect(eventCalls.length).to.equal(1); expect(eventCalls[0].name).to.include('method=email');