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

Speed Up Internals #499

Open
danpastori opened this issue May 4, 2022 Discussed in #498 · 0 comments · May be fixed by #514
Open

Speed Up Internals #499

danpastori opened this issue May 4, 2022 Discussed in #498 · 0 comments · May be fixed by #514
Assignees
Labels

Comments

@danpastori
Copy link
Contributor

danpastori commented May 4, 2022

Discussed in #498

Originally posted by danpastori May 4, 2022

👉 Describe the problem

We want Amplitude to be lightweight, fast, and efficient. Some new Javascript features and and more experience allow for these upgrades to come to light.

👥 Problem evidence & reach

Everyone using AmplitudeJS

🥰 Describe the "impact" on users?

AmplitudeJS will perform better, be quicker, and more lightweight

🏆 How to solve this problem

Priority 10/10: Update tests to Vitest

Vitest offers the latest and greatest testing suite. This will make AmplitudeJS more bulletproof and offer better integration testing: https://vitest.dev/

Priority 10/10: Update build to WebPack 5

Updating to WebPack 5 allows for better builds and more efficient configuration: https://webpack.js.org/concepts/

Priority 9/10: Only Bind Elements Being Used

We can query any element that starts with the data-amplitude-* prefix and bind what's necessary. This way, if you aren't using all of the elements, then we don't waste time looking for them.

Priority 9/10: Use Array methods like .map(), .reduce(), .filter(), etc.

Less looping, smaller code base. Just make things easier to manage.

Priority 8/10: Change from .classList.remove() and .classList.add() to .classList.toggle()

Less code to do what we were doing before, win win.

Priority 5/10: Do not run initializations unless there is something to initialize.

Right now we run all of the initializations even if there isn't something defined by the user. Make sure we don't run anything we don't have to.

Priority 1/10: Research element.contains() for click/touch handlers

This will allow you to monitor interactions within an element instead of only on the element itself.
Allows propagation from inside an element to be handled correctly with AmplitudeJS. Specifically on play button images.

This might not be a big deal, but I know we ran into issues with this before.

💯 How do we validate the problem is solved?

AmplitudeJS is faster and more lightweight than before while maintaining existing features.

@danpastori danpastori added the Enhancement ⚡️ New feature label May 4, 2022
@danpastori danpastori self-assigned this May 4, 2022
@danpastori danpastori moved this to Ready for Development in AmplitudeJS 6.0 May 4, 2022
@danpastori danpastori linked a pull request Jun 3, 2022 that will close this issue
75 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Ready for Development
Development

Successfully merging a pull request may close this issue.

1 participant