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

feat: speed up objects loops #5

Conversation

simone-sanfratello
Copy link

Copy link
Owner

@AsyncBanana AsyncBanana left a comment

Choose a reason for hiding this comment

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

LGTM

@AsyncBanana
Copy link
Owner

Thanks for the PR!
Because of the added size, I decided not to merge this for now. However, if you have any ideas for reducing the size while keeping the speed advantage of this, I would be happy to look at this again.

@simone-sanfratello
Copy link
Author

if size is very important, I recommend to use uglify on the build script

uglifyjs dist/index.js -c -m > dist/index-compres.js

@AsyncBanana
Copy link
Owner

I was considering that when I started the project, but I was more focused on size for web projects, as this project is for both the web and Node. Most people using it on the web will probably use minifiers themselves, and many people using Node like seeing the code in their dependencies directly.
I started #8 to see the community's opinions on this. You can add your own opinion if you want.

@danialdezfouli
Copy link
Contributor

using Array.length in for loops has a performance cost; you can save it before the loop, like this:
const keysLength = keys.length;

@AsyncBanana
Copy link
Owner

I am closing this because I will not implement this unless something changes my mind.

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