Skip to content

Collect timezone and allow userAgentData to be overridden#956

Merged
danieljackins merged 4 commits intomasterfrom
collect-timezone
Sep 26, 2023
Merged

Collect timezone and allow userAgentData to be overridden#956
danieljackins merged 4 commits intomasterfrom
collect-timezone

Conversation

@danieljackins
Copy link
Copy Markdown
Contributor

@danieljackins danieljackins commented Sep 21, 2023

  • I've included a changeset (psst. run yarn changeset. Read about changesets here).

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Sep 21, 2023

🦋 Changeset detected

Latest commit: fedad18

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@segment/analytics-next Minor

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
Copy link
Copy Markdown
Contributor

silesky commented Sep 21, 2023

@danieljackins CI is failing FYI

Comment on lines +219 to +220
evtCtx.userAgentData = await clientHints(
this.instance.options.highEntropyValuesClientHints
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.

Similar to the original PR, can we cache the client hints during the plugin load? These fields won't change, so no reason to perform a potentially expensive operation on every event 😄


it('should add .timezone', async () => {
const ctx = await analytics.track('test')
assert(ctx.event.context?.timezone)
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.

can we assert that this is a string / actual TZ?

Copy link
Copy Markdown
Contributor

@silesky silesky left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@chrisradek chrisradek left a comment

Choose a reason for hiding this comment

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

:shipit:

@danieljackins danieljackins merged commit f5cdb82 into master Sep 26, 2023
@danieljackins danieljackins deleted the collect-timezone branch September 26, 2023 18:22
@github-actions github-actions bot mentioned this pull request Sep 26, 2023
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.

3 participants