-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ServerSideRender: Use data hooks instead of HoC #70529
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
Conversation
Size Change: -24 B (0%) Total Size: 1.86 MB
ℹ️ View Unchanged
|
Flaky tests detected in d980099. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/15897914343
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors the ExportedServerSideRender
component to use useSelect
and useMemo
hooks instead of the withSelect
higher-order component, streamlining the component tree and improving performance.
- Converted from HOC to function component with hooks
- Replaced
withSelect
withuseSelect
for fetchingcurrentPostId
- Updated
urlQueryArgs
computation to useuseMemo
Comments suppressed due to low confidence (1)
packages/server-side-render/src/index.js:17
- Consider adding unit tests for
ExportedServerSideRender
to verify behavior whencurrentPostId
isnull
versus a number, ensuring both branches of the URL args merging logic are covered.
export default function ExportedServerSideRender( {
return EMPTY_OBJECT; | ||
} )( ( { urlQueryArgs = EMPTY_OBJECT, currentPostId, ...props } ) => { | ||
return postId && typeof postId === 'number' ? postId : null; | ||
}, [] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing an empty dependency array to useSelect
prevents it from updating when the editor store changes. Remove the []
so that useSelect
re-runs on relevant store updates and keeps currentPostId
in sync.
}, [] ); | |
} ); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect 😄 An empty dependency array doesn't prevent useSelect
from listening to the store updates.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
What?
PR refactors
ExportedServerSideRender
component to use data hooks instead of HoCs.Why?
A micro-optimization reduces the size of the rendered component tree.
Testing Instructions