Skip to content
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

chore: add deprecation note for DNT #4020

Merged
merged 7 commits into from
Jun 4, 2024
Merged

chore: add deprecation note for DNT #4020

merged 7 commits into from
Jun 4, 2024

Conversation

samisayegh
Copy link
Contributor

@samisayegh samisayegh commented May 28, 2024

The PR adds a deprecation note to remove DNT as part of the next major release.
The decision is captured here. The main drivers were:

  • DNT is an outdated standard. Newer standards like GPC have been introduced. We don't believe it's Coveo's role to update libraries to adhere to the latest standards?
  • Privacy standards differ around the world. Baking a single privacy stance into Coveo's libraries and apis will be overly restrictive in certain geographies.

Aside
I tried to adjust the logic in headless to only apply DNT if the analyticsMode is legacy. However, this required changing an action to an thunk. The signature change created multiple typescript errors, and so seems like it's breaking. I changed course to add a deprecation note instead. With the v3 release around the corner, it seems less risky to simply remove the doNotTrack function rather than making a signature change.

@samisayegh samisayegh requested a review from sallainCoveo May 28, 2024 22:11
@samisayegh samisayegh requested a review from a team as a code owner May 28, 2024 22:11
@olamothe
Copy link
Member

I tried to adjust the logic in headless to only apply DNT if the analyticsMode is legacy.

Here is what I would propose to try:

  • In configuration-actions -> updateAnalyticsConfiguration() -> remove the doNotTrack() function call.
  • In engine.ts -> getUpdateAnalyticsConfigurationPayload(), call doNotTrack() only if configuration.analytics.analyticsMode === 'legacy'

When you look at the full logic, calling doNotTrack() in updateAnalyticsConfiguration() is a bit of a suspender with belt thingy.

@developer-experience-bot
Copy link
Contributor

developer-experience-bot bot commented May 31, 2024

Pull Request Report

PR Title

✅ Title follows the conventional commit spec.

Live demo links

Bundle Size

File Old (kb) New (kb) Change (%)
case-assist 202.8 202.9 0
commerce 285.9 285.9 0
search 367.4 367.4 0
insight 347.7 347.7 0
product-listing 262.2 262.2 0
product-recommendation 171.9 171.9 0
recommendation 215.8 215.8 0
ssr 360.2 360.2 0

SSR Progress

Use case SSR (#) CSR (#) Progress (%)
search 39 44 89
recommendation 0 4 0
product-recommendation 0 10 0
product-listing 0 13 0
case-assist 0 6 0
insight 0 27 0
commerce 0 7 0
Detailed logs search : buildInteractiveResult
search : buildInteractiveInstantResult
search : buildInteractiveRecentResult
search : buildInteractiveCitation
search : buildGeneratedAnswer
recommendation : missing SSR support
product-recommendation : missing SSR support
product-listing : missing SSR support
case-assist : missing SSR support
insight : missing SSR support
commerce : missing SSR support

@samisayegh
Copy link
Contributor Author

Made the changes @olamothe. Removing doNotTrack from the action simplifies the problem, but is a change in behavior. Are we ok with this?

@samisayegh samisayegh added this pull request to the merge queue Jun 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 3, 2024
@samisayegh samisayegh added this pull request to the merge queue Jun 4, 2024
Merged via the queue into master with commit 41efb66 Jun 4, 2024
92 checks passed
@samisayegh samisayegh deleted the LENS-1891 branch June 4, 2024 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants