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

SearchControl: Fix rest props mutation #65740

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Conversation

tyxla
Copy link
Member

@tyxla tyxla commented Sep 30, 2024

What?

Currently, we're mutating a rest props object in the SearchControl component. This PR refactors to remove that mutation.

Why?

By mutating a rest props object, we could easily be mutating props themselves, and mutating props is not recommended as it could lead to unexpected consequences.

Also, this resolves an ESLint error when using React Compiler:

/gutenberg/packages/components/src/search-control/index.tsx
70:9  error  Mutating component props or hook arguments is not allowed. Consider using a local variable instead  react-compiler/react-compiler

See #61788 for more details.

How?

We're using an extra rest props call to avoid the mutation.

I believe no CHANGELOG is necessary in this PR.

Testing Instructions

Test SearchControl in Storybook and verify there are no regressions.

Testing Instructions for Keyboard

Same

Screenshots or screencast

None

@tyxla tyxla added [Type] Code Quality Issues or PRs that relate to code quality [Package] Components /packages/components labels Sep 30, 2024
@tyxla tyxla requested review from Mamaduka and a team September 30, 2024 12:15
@tyxla tyxla self-assigned this Sep 30, 2024
Copy link

github-actions bot commented Sep 30, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @tyxla!

@tyxla tyxla merged commit aa2c25a into trunk Oct 1, 2024
72 of 73 checks passed
@tyxla tyxla deleted the fix/search-control-prop-mutation branch October 1, 2024 06:55
@github-actions github-actions bot added this to the Gutenberg 19.4 milestone Oct 1, 2024
getdave pushed a commit that referenced this pull request Oct 1, 2024
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
huubl pushed a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
huubl added a commit to huubl/gutenberg that referenced this pull request Oct 2, 2024
draganescu pushed a commit that referenced this pull request Oct 2, 2024
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants