Skip to content

Commit 92eaf53

Browse files
committed
Resolve PR comments
1 parent faf051f commit 92eaf53

File tree

6 files changed

+19
-23
lines changed

6 files changed

+19
-23
lines changed

.changeset/silly-panthers-mix.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
'@firebase/analytics': minor
33
---
44

5-
Add function setDefaultEventParameters() to set data that will be logged on every analytics SDK event
5+
Add function `setDefaultEventParameters()` to set data that will be logged on every analytics SDK event

packages/analytics/src/api.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,10 @@ export function setAnalyticsCollectionEnabled(
227227
}
228228

229229
/**
230-
* Adds data that will be set on every event logged from the SDK, including automatic one.
231-
* By using gtag's set command, the values passed persist on the current page and are passed with
230+
* Adds data that will be set on every event logged from the SDK, including automatic ones.
231+
* By using gtag's "set" command, the values passed persist on the current page and are passed with
232232
* all subsequent events.
233-
*
233+
* @public
234234
* @param customParams Any custom params the user may pass to gtag.js.
235235
*/
236236
export function setDefaultEventParameters(customParams: CustomParams): void {

packages/analytics/src/functions.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ describe('FirebaseAnalytics methods', () => {
180180
_setDefaultEventParametersForInit(eventParametersForInit);
181181
expect(defaultEventParametersForInit).to.deep.equal(eventParametersForInit);
182182
});
183-
it('_setDefaultEventParametersForInit() adds new params correctly', async () => {
183+
it('_setDefaultEventParametersForInit() replaces previous params with new params', async () => {
184184
const eventParametersForInit = {
185185
'github_user': 'dwyfrequency',
186186
'company': 'google'
@@ -189,7 +189,6 @@ describe('FirebaseAnalytics methods', () => {
189189
_setDefaultEventParametersForInit(eventParametersForInit);
190190
_setDefaultEventParametersForInit(additionalParams);
191191
expect(defaultEventParametersForInit).to.deep.equal({
192-
...eventParametersForInit,
193192
...additionalParams
194193
});
195194
});

packages/analytics/src/functions.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,20 +150,13 @@ export async function setAnalyticsCollectionEnabled(
150150
}
151151

152152
/**
153-
* Sets the variable {@link defaultEventParametersForInit} for use in the initialization of
153+
* Sets the variable `defaultEventParametersForInit` for use in the initialization of
154154
* analytics.
155155
*
156156
* @param customParams Any custom params the user may pass to gtag.js.
157157
*/
158158
export function _setDefaultEventParametersForInit(
159-
customParams: CustomParams
159+
customParams?: CustomParams
160160
): void {
161-
if (defaultEventParametersForInit) {
162-
defaultEventParametersForInit = {
163-
...defaultEventParametersForInit,
164-
...customParams
165-
};
166-
} else {
167-
defaultEventParametersForInit = customParams;
168-
}
161+
defaultEventParametersForInit = customParams;
169162
}

packages/analytics/src/initialize-analytics.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { Deferred } from '@firebase/util';
3030
import { _FirebaseInstallationsInternal } from '@firebase/installations';
3131
import { removeGtagScript } from '../testing/gtag-script-util';
3232
import { setDefaultEventParameters } from './api';
33+
import { defaultEventParametersForInit } from './functions';
3334

3435
const fakeMeasurementId = 'abcd-efgh-ijkl';
3536
const fakeFid = 'fid-1234-zyxw';
@@ -100,11 +101,11 @@ describe('initializeAnalytics()', () => {
100101
});
101102
it('calls gtag set if there are default event parameters', async () => {
102103
stubFetch();
103-
const defaultEventParametersForInit = {
104+
const eventParametersForInit = {
104105
'github_user': 'dwyfrequency',
105106
'company': 'google'
106107
};
107-
setDefaultEventParameters(defaultEventParametersForInit);
108+
setDefaultEventParameters(eventParametersForInit);
108109
await _initializeAnalytics(
109110
app,
110111
dynamicPromisesList,
@@ -113,10 +114,9 @@ describe('initializeAnalytics()', () => {
113114
gtagStub,
114115
'dataLayer'
115116
);
116-
expect(gtagStub).to.be.calledWith(
117-
GtagCommand.SET,
118-
defaultEventParametersForInit
119-
);
117+
expect(gtagStub).to.be.calledWith(GtagCommand.SET, eventParametersForInit);
118+
// defaultEventParametersForInit is reset after initialization.
119+
expect(defaultEventParametersForInit).to.equal(undefined);
120120
});
121121
it('puts dynamic fetch promise into dynamic promises list', async () => {
122122
stubFetch();

packages/analytics/src/initialize-analytics.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ import {
2828
import { ERROR_FACTORY, AnalyticsError } from './errors';
2929
import { findGtagScriptOnPage, insertScriptTag } from './helpers';
3030
import { AnalyticsSettings } from './public-types';
31-
import { defaultEventParametersForInit } from './functions';
31+
import {
32+
defaultEventParametersForInit,
33+
_setDefaultEventParametersForInit
34+
} from './functions';
3235

3336
async function validateIndexedDB(): Promise<boolean> {
3437
if (!isIndexedDBAvailable()) {
@@ -145,6 +148,7 @@ export async function _initializeAnalytics(
145148
// Detects if there is data that will be set on every event logged from the SDK.
146149
if (defaultEventParametersForInit) {
147150
gtagCore(GtagCommand.SET, defaultEventParametersForInit);
151+
_setDefaultEventParametersForInit(undefined);
148152
}
149153

150154
return dynamicConfig.measurementId;

0 commit comments

Comments
 (0)