Skip to content

Forward undefined to target props instead of bool #878

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

Merged
merged 2 commits into from
Jul 31, 2022

Conversation

schmijos
Copy link
Contributor

react-dom complains with a warning if it receives booleans for non-boolean DOM properties (see https://github.com/facebook/react/blob/main/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js#L186-L190).

This passes undefined to the target props instead of `false to fix the warning.

CleanShot 2022-07-13 at 15 12 30@2x

@tcbegley
Copy link
Collaborator

Thanks @schmijos, it will be a week or so before I can look at this properly, but I appreciate you taking the time to submit!

_react-dom_ complains with a warning if it receives
booleans for non-boolean DOM properties (see
<https://github.com/facebook/react/blob/main/packages/react-dom/src/shared/ReactDOMUnknownPropertyHook.js#L186-L190>).

This passes `undefined` to the `target` props instead of `false to
fix the warning.
@schmijos schmijos force-pushed the fix-bool-attr-warning branch from f8240c9 to 2b80428 Compare July 17, 2022 09:27
Copy link
Collaborator

@tcbegley tcbegley left a comment

Choose a reason for hiding this comment

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

Thanks for this @schmijos!

Sorry for the slow response, this is a project I maintain in my spare time which has been lacking the last few weeks for various reasons. Appreciate the contribution!

@tcbegley tcbegley merged commit f78bdb3 into facultyai:main Jul 31, 2022
This was referenced Jul 31, 2022
@schmijos schmijos deleted the fix-bool-attr-warning branch August 5, 2022 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants