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

Data: Remove unused forceRender argument #19206

Merged
merged 1 commit into from Dec 17, 2019
Merged

Conversation

@aduth
Copy link
Member

aduth commented Dec 17, 2019

This pull request seeks to remove an unused argument in the implementation of useSelect. When a change is detected in the result of a mapSelect callback, the hook will force a render using its internal forceRender reducer. Since forceRender is implemented using a useReducer which will always yield a new value and never considers the incoming argument, the empty object we currently pass is unused.

const [ , forceRender ] = useReducer( ( s ) => s + 1, 0 );

In theory, the previous implementation could cause memory accumulation and/or extra load on garbage collection due to the initialization and subsequent destruction of the unused object references. The expected impact of this is not significant, but since this is a hot code path, it is worth the extra attention.

I expect this could have been lingering code from an earlier iteration of the hook, considering that this pattern is often used when forcing a render using the setState callback of a useState hook (where setState( {} ) is equally as effective at forcing a render).

Testing Instructions:

Ensure unit tests pass:

npm run test-unit
@nerrad
nerrad approved these changes Dec 17, 2019
Copy link
Contributor

nerrad left a comment

Oh, good catch!

@aduth aduth merged commit b45e53e into master Dec 17, 2019
2 checks passed
2 checks passed
pull-request-automation
Details
Travis CI - Pull Request Build Passed
Details
@aduth aduth deleted the remove/use-select-force-render-arg branch Dec 17, 2019
@youknowriad youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.