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

Emit Reset Events #704

Merged
merged 3 commits into from
Nov 30, 2022
Merged

Emit Reset Events #704

merged 3 commits into from
Nov 30, 2022

Conversation

pooyaj
Copy link
Contributor

@pooyaj pooyaj commented Nov 30, 2022

  • Emitting a reset event when user calls analytics.reset(). This allows for external code to hook into the reset events and do further logic.
  • I've included a changeset (psst. run yarn changeset. Read about changesets here).

@changeset-bot
Copy link

changeset-bot bot commented Nov 30, 2022

🦋 Changeset detected

Latest commit: 9399b31

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

@pooyaj pooyaj requested a review from a team November 30, 2022 00:51
@silesky
Copy link
Contributor

silesky commented Nov 30, 2022

Looks good. Seems like the event emitter was kind of an afterthought, when it really can be exceptionally useful (I don't even know fits documented in the browser?) -- in node, the contract is very well defined and fully typed and every event gets emitted on by default with the context as an argument:

emitter.emit(event.type, ctx)

^ creating an enhanced dispatch feels cleaner, so we would just automatically emit on every type of event. We might be able to just port that function wholesale.

@pooyaj
Copy link
Contributor Author

pooyaj commented Nov 30, 2022

Yeah, agreed. The browser version emits separately in each of the track/identify/etc. calls with different parameters passed as extra context.

@pooyaj pooyaj merged commit 6e42f6e into master Nov 30, 2022
@pooyaj pooyaj deleted the pj/reset-dispatcher branch November 30, 2022 17:55
@github-actions github-actions bot mentioned this pull request Nov 30, 2022
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.

2 participants