-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: adds the use of topsort sdk for reporting events #275
Conversation
4abf986
to
099ec6a
Compare
099ec6a
to
5b8278b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. Thanks for the change. My only concern is about the resulting library size and whether they builder is actually applying tree shaking for the dependencies.
Could you add to the PR description the size of the library before and after the change.
I would like to understand why the size grows. Since its so small, can you peek into the diff? |
The difference comes from:
Please feel free to check the diff here as well (also made it part of the description) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Lets wait for @sk- as well.
This PR adds the use of Topsort.js SDK for reporting events
You can see the use inside analytics in this video
The difference comes from:
withValidation
(HOF used to wrap functions that we will validate fields)Please feel free to check the diff here as well