Consent - Do not load segment/send events unneccessarily #1001
Conversation
🦋 Changeset detectedLatest commit: 69bf193 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
silesky
changed the title
Will not? |
| @@ -13,7 +13,7 @@ | |||
| "!**/test-helpers/**" | |||
| ], | |||
| "scripts": { | |||
| ".": "yarn run -T turbo run --filter=@segment/analytics-consent-tools", | |||
| ".": "yarn run -T turbo run --filter=@segment/analytics-consent-tools...", | |||
There was a problem hiding this comment.
Just curious, when is this script run?
There was a problem hiding this comment.
It's just a helper for local development. I think I wrote about in the docs under development. This makes it useful for "yarn . watch" (watch this and all of its dependencies).
Also, ...package-name watches dependents -- which might also be a useful alias. e.g "yarn ... watch"
There was a problem hiding this comment.
Actually, I take that back about local development: we do use this in CI to lazily build specific packages. :-)
| const isDisabled = ( | ||
| cdnSettings: CDNSettings, | ||
| initialCategories: Categories, | ||
| disableOpt: InitOptions['disable'] | undefined |
There was a problem hiding this comment.
Should we have this shortcircuit before everything? As in, if disable is true then bypass all the cdnsettings function
There was a problem hiding this comment.
I actually was in the process of refactoring this when you wrote this comment =)
| import { segmentShouldBeDisabled } from '../disable-segment' | ||
|
|
||
| describe('segmentShouldBeDisabled', () => { | ||
| it('should be disabled if user has only consented to irrelevant vategories', () => { |
See changeset.
This will not cause any problems for recent older versions of analytics that don't have "disabled" -- they just won't get this feature.