Skip to content

Change default tracker initialization parameters for JS/Browser v4 - #1144

Merged
Peter Perlepes (igneel64) merged 1 commit into
release/4.0.0from
feature/v4-change-default-initialization-params-browser
Mar 9, 2023
Merged

Change default tracker initialization parameters for JS/Browser v4#1144
Peter Perlepes (igneel64) merged 1 commit into
release/4.0.0from
feature/v4-change-default-initialization-params-browser

Conversation

@igneel64

Copy link
Copy Markdown
Contributor

This PR includes the proposed changes for the JS/Browser tracker initialization parameters:

Currently it includes:

  • discoverRootDomain: The default becomes true
  • configCookieSameSite: The default becomes Lax

A bit more info:

discoverRootDomain

On the previous version this value defaulted to false.

This had an important effect when used as a default. If the tracker was installed in a subdomain (foo.example.com), or any number of subdomains (bar.example.com etc), and a TLD (example.com), then all instances had different domain userIds.

That would make the analysis a bit more challenging when trying to count unique users, without the existance of user ID (uid), and cross TLD-subdomain behaviours/attribution.

The behaviour now mimics what other tools like Google Analytics have as a default.

configCookieSameSite

This was set as the recommended value of Lax but defaults to None which seems to have caused some issues for unsupported browsers and tracking use cases.
Since our v4 compatibility does not allow us to follow the changes made in latest browser versions, this would be a good default to follow for this version:
More info:

@bundlemon

bundlemon Bot commented Jan 25, 2023

Copy link
Copy Markdown

BundleMon

Files added (6)
Status Path Size Limits
libraries/browser-tracker-core/dist/index.mod
ule.js
+23.87KB 25KB / +10%
trackers/javascript-tracker/dist/sp.js
+22.49KB 25KB / +10%
trackers/javascript-tracker/dist/sp.lite.js
+13.93KB 15KB / +10%
trackers/browser-tracker/dist/index.umd.min.j
s
+13.78KB 15KB / +10%
libraries/tracker-core/dist/index.module.js
+13.36KB 15KB / +10%
trackers/browser-tracker/dist/index.module.js
+3.46KB 5KB / +10%

Total files change +90.88KB 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice change! 👍

I can't comment in the file directly, but can you fix the API comments in the types.ts file? It still says that the default for discoverRootDomain is false and for cookieSameSite is None.

@igneel64

Copy link
Copy Markdown
Contributor Author

Sure thanks!

@paulboocock

Copy link
Copy Markdown

On discoverRootDomain, I've never been a big fan of how it tries to keep setting cookies. This usually leads of console warnings, which is always unfortunate. I'd prefer to not have a default value which commonly produces warnings.

However, I do see the benefit in making the default cookie domain the root... maybe we can try and find it another way - trim window.location to the typical value and try to set the cookie? Maybe a little regex solves this. Worst case, we fallback to cycle through the other options if we don't find the root straight away?

@igneel64

Copy link
Copy Markdown
Contributor Author

Paul Boocock (@paulboocock)
Summing up, together with this change we can refine a bit the way we try to find the root right ? (I think it can fit this PR and can be quite straightforward)

@igneel64
Peter Perlepes (igneel64) force-pushed the feature/v4-change-default-initialization-params-browser branch from ecd050c to 9ba4395 Compare January 26, 2023 08:55
@igneel64
Peter Perlepes (igneel64) force-pushed the release/4.0.0 branch 3 times, most recently from 579acd6 to 4ea6b1f Compare February 6, 2023 11:08
@igneel64

Copy link
Copy Markdown
Contributor Author

On discoverRootDomain, I've never been a big fan of how it tries to keep setting cookies. This usually leads of console warnings, which is always unfortunate. I'd prefer to not have a default value which commonly produces warnings.

However, I do see the benefit in making the default cookie domain the root... maybe we can try and find it another way - trim window.location to the typical value and try to set the cookie? Maybe a little regex solves this. Worst case, we fallback to cycle through the other options if we don't find the root straight away?

Addressed at #1163

@igneel64
Peter Perlepes (igneel64) force-pushed the feature/v4-change-default-initialization-params-browser branch from 9ba4395 to 8fe93d8 Compare March 7, 2023 09:33
@igneel64
Peter Perlepes (igneel64) merged commit 8274cb3 into release/4.0.0 Mar 9, 2023
@igneel64
Peter Perlepes (igneel64) deleted the feature/v4-change-default-initialization-params-browser branch March 9, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants