Skip to content

Fix: ToolsPanelItem: hasValue silently fails when it returns a non-boolean (e.g., undefined), causing the control to not render - #73069 #73074

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.

Sign up for GitHub

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

Jump to bottom
Open
hbhalodia wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Fix: ToolsPanelItem: hasValue silently fails when it returns a non-boolean (e.g., undefined), causing the control to not render - #73069 #73074

hbhalodia wants to merge 2 commits into from

Conversation

@hbhalodia
Copy link
Contributor

@hbhalodia hbhalodia commented Nov 7, 2025

What?

Closes #73069

Why?

  • When passing an undefined value to hasValue prop in ToolsPanelItem component, the component was not being rendered and it does not reproduce any console erorr, so user/dev can know the exact issue.

How?

  • The current approach, changes evaluate the value for hasValue prop to Boolean and does not rely on user passed input so that the component can render without any issue.
  • Since if user/dev can pass undefined or any falsy or truthy value, the implementation intercepts and convert to actual Boolean True and False, such that it does not fails to render component.
  • This approach does not need any console/log to show user about the issue because ultimately user supplied value and hasValue prop value should be Boolean.

Testing Instructions

<ToolsPanelItem
  hasValue={() => hoge || foo}
  /* ... */
>
  {/* control */}
</ToolsPanelItem>

Add the implementation for above component, you will notice that it would render component.

Testing Instructions for Keyboard

  • None.

@hbhalodia hbhalodia self-assigned this Nov 7, 2025
@hbhalodia hbhalodia requested a review from ajitbohra as a code owner November 7, 2025 08:17
@hbhalodia hbhalodia added [Type] Bug An existing feature does not function as intended [Package] Components /packages/components labels Nov 7, 2025
@hbhalodia hbhalodia mentioned this pull request Nov 7, 2025
Open
6 tasks
@github-actions
Copy link

github-actions bot commented Nov 7, 2025 •
edited
Loading

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.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @ddryo.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

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

Unlinked contributors: ddryo.

Co-authored-by: hbhalodia <hbhalodia@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@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.

@aaronrobertshaw
Copy link
Contributor

aaronrobertshaw commented Nov 10, 2025

I'm not opposed to these changes but I don't think they are 100% necessary.

I think there might have been a misunderstanding about the expected behaviour of the ToolsPanelItem component, see #73069 (comment) for more details.

aaronrobertshaw
aaronrobertshaw reviewed Nov 10, 2025
View reviewed changes
packages/components/src/tools-panel/tools-panel-item/hook.ts
// dependency to the useCallback hook! If needed, we should use a ref.
const hasValueCallback = useCallback( hasValue, [ panelId ] );
const hasValueCallback = useCallback(
() => Boolean( hasValue() ),
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw Nov 10, 2025

Choose a reason for hiding this comment

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

I'm not sure casting this everywhere adds much.

The types define that hasValue should return a boolean so invalid props there should probably be caught during development. If they aren't the result without casting to a boolean is still functionally correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aaronrobertshaw aaronrobertshaw aaronrobertshaw left review comments

@ajitbohra ajitbohra Awaiting requested review from ajitbohra ajitbohra is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@hbhalodia hbhalodia

Labels

[Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ToolsPanelItem: hasValue silently fails when it returns a non-boolean (e.g., undefined), causing the control to not render

2 participants

@hbhalodia @aaronrobertshaw