-
-
Notifications
You must be signed in to change notification settings - Fork 244
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
feat: initialize server side google analytics with multiple options #64
base: master
Are you sure you want to change the base?
feat: initialize server side google analytics with multiple options #64
Conversation
* @param {string} pluginConfig.trackingId - Google Analytics site tracking Id. Same as tid (se below), only one needed | ||
* @param {string} pluginConfig.tid - Google Analytics site tracking Id. Same as trackingId (se above), only one needed | ||
* @param {string} [pluginConfig.cid] - Google Analytics client Id. It generates a random UUID if none is set | ||
* @param {boolean} [pluginConfig.strictCidFormat] - Set it to false to set a custom client Id (not UUID). Only used if cid is set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like strictCidFormat
is used. Do we need that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -7,12 +7,24 @@ if (!process.browser) { | |||
/** | |||
* Serverside Google Analytics plugin | |||
* @param {object} pluginConfig - Plugin settings | |||
* @param {string} pluginConfig.trackingId - Google Analytics site tracking Id | |||
* @param {string} pluginConfig.trackingId - Google Analytics site tracking Id. Same as tid (se below), only one needed | |||
* @param {string} pluginConfig.tid - Google Analytics site tracking Id. Same as trackingId (se above), only one needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tid
property looks like a repeat of trackingId
do we need that or can we just use the trackingId
field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it is indeed.
I added this 4 new docummented attributes of pluginConfig
but there are more that can be used in universal-analytics
initialization.
I keeped the trackingId
attribute in order to keep compatibility with the actual implementation and also allow to define all the attributes accepted in universal-analytics
initialization.
Maybe it would be better to keep the actual jsDoc and add only a reference to all the possible attributes that can be used in pluginConfig
and explaining that the trackingId
attribute can be declared also as tid
.
Thanks for the PR! ❤️ Added a couple of notes |
Update for Google Analytics plugin
When tracking from server side sometimes is needed to share the
clientId
created in the browser so the tracking on both sides (browser and server) represents the flow done by the same user.Now, the Google Analytics can be initialized on the server side only with the
trackingId
, not being able to specify aclientId
, but theuniversal-analytics
library accepts more options on its initialization, including theclientId
,userId
, etc.This PR enables the possibility of initializing the Google Analytics on the server side with all the possible options that
universal-analytics
accepts and also mantaining the actual configuration.