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

Memoize inner selector function #410

Merged
merged 2 commits into from
Jun 2, 2018

Conversation

markwhitfeld
Copy link
Member

No description provided.

Mark Whitfeld added 2 commits June 2, 2018 23:12
Also change to a single value cache for performance
The deep cache isn't used anyway
@markwhitfeld markwhitfeld requested a review from amcdnl June 2, 2018 21:14
return false;
}

// Do this in a for loop (and not a `forEach` or an `every`) so we can determine equality as fast as possible.
Copy link
Member

Choose a reason for hiding this comment

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

I thought you could break out of a forEach by returning false. Just a side note.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the code from the reselect redux library, so I assume it was done this way for fine tuned performance

Copy link
Member

Choose a reason for hiding this comment

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

Ya, while used to be fastest but I think thats changed recently. https://jsperf.com/for-vs-foreach/333

Copy link
Member

@amcdnl amcdnl left a comment

Choose a reason for hiding this comment

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

LGTM

@amcdnl amcdnl merged commit 456df13 into ngxs:master Jun 2, 2018
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.

None yet

2 participants