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

Fix reduce function being used incorrectly in core app path #306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

atwalg2
Copy link

@atwalg2 atwalg2 commented Aug 20, 2024

Description

Note: This PR improves performance

My goal here is to improve the code base, I noticed a bad practice happening in the code base and wanted to help. Also it could potentially hurt performance if this is used a lot in someone's code base.

The issue is how Javascript reduce is being used

When using reduce, you are allowed to use mutation when you just building from a new object, especially a place empty object as your initial value, this is because it is impossible to have a side effect in that case. You are simply building an object while assigning properties one by one in these examples.. However, you are creating new memory every single loop when you use the spread operator this way.

Reviewers’ hat-rack 🎩

This changes no behavior

  • All tests still pass

@atwalg2
Copy link
Author

atwalg2 commented Aug 20, 2024

I have signed the CLA!

@flodlc
Copy link

flodlc commented Aug 27, 2024

Good catch!
Did you make any benchmark to check if there is a notable perf improvement ?

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