react
react copied to clipboard
facebook
React DOM: Support boolean values for `inert` prop
Stacked on #27883 (Diff against #27883)
Implementation alternates:
- 78f1ad60b830065329e4383d23afb1dd2ddc2378
- 78391f0872583611dcb94f958815f5e43ae4c094
- 1e928d831fe143d1ae3e5a2d88cdac8b27fbdcc1
3 leverages fallthroughs more but requires dropping the default case in favor of early return. I don't know how else I can make it work with the flag. basically, how to implement
function isBooleanProp(enableNewBooleanProps, prop) {
switch (prop) {
case 'scoped':
if (!enableNewBooleanProps) {
return true
}
// fallthrough with enableNewBooleanProps
case 'inert':
if (enableNewBooleanProps) {
return true
} else {
// Goto default how?
}
// We should not fallthrough here
case 'download':
// previous cases cannot fallthrough into this
return 'overloaded'
default:
return false
}
}
Summary
Adds support for HTMLElement.inert behind enableNewBooleanProps which is turned on in experimental builds.
Note that the previous workaround (inert="") will no longer work since the empty string is considered false for boolean props.
Closes https://github.com/facebook/react/issues/17157
How did you test this change?
You need Chrome >=102 (or any supporting browser listed in https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/inert#browser_compatibility).
- [x] Attribute fixture (commited changes with non-experimental build since this is the build we were using before)
- [x] Codesandbox from repro will contain matching behavior of DOM and React: https://codesandbox.io/p/sandbox/react-inert-pr-83ymvy
Note that the previous workaround (inert="") will no longer work since the empty string is considered false for boolean props.
Seems like we might want to add this as a breaking change for 19 then? Maybe add it behind a flag in experimental for now so we know to enable it later?
Seems like we might want to add this as a breaking change for 19 then? Maybe add it behind a flag in experimental for now so we know to enable it later?
Yeah that seems reasonable.
Though we'll run into this scenario over and over and it's a bit annoying for React users since inert is part of the standard and supported by now.
Would it make sense for unknown props to check if a prop is an IDL attribute (e.g. 'inert' in htmlElement) and then set the IDL attribute to the prop value? Relying on the fact the browser is responsible for potentially reflecting the value in the content attribute. This would match the behavior behind enableCustomElementPropertySupport for custom component tags.
Comparing: 4cd788aef03d8f32c03e4dac4d0cf28b220cedfb...b052e933168500975370cd1ea2e39a2840e93bc3
Critical size changes
Includes critical production bundles, as well as any change greater than 2%:
| Name | +/- | Base | Current | +/- gzip | Base gzip | Current gzip |
|---|---|---|---|---|---|---|
| oss-stable/react-dom/cjs/react-dom.production.min.js | = | 131.79 kB | 131.79 kB | = | 42.40 kB | 42.40 kB |
| oss-experimental/react-dom/cjs/react-dom.production.min.js | +0.02% | 137.06 kB | 137.08 kB | +0.04% | 44.00 kB | 44.01 kB |
| facebook-www/ReactDOM-prod.classic.js | = | 457.17 kB | 457.17 kB | = | 83.22 kB | 83.22 kB |
| facebook-www/ReactDOM-prod.modern.js | +0.01% | 442.41 kB | 442.47 kB | +0.03% | 80.94 kB | 80.96 kB |
| facebook-www/ReactDOMForked-prod.classic.js | = | 457.94 kB | 457.94 kB | = | 83.33 kB | 83.33 kB |
Significant size changes
Includes any change greater than 0.2%:
Expand to show
| Name | +/- | Base | Current | +/- gzip | Base gzip | Current gzip |
|---|---|---|---|---|---|---|
| facebook-www/ReactFlightDOMRelayServer-prod.modern.js | +0.29% | 27.91 kB | 28.00 kB | +0.31% | 7.37 kB | 7.40 kB |
Generated by :no_entry_sign: dangerJS against b052e933168500975370cd1ea2e39a2840e93bc3
@sebmarkbage I could add hidden to OVERLOADED_BOOLEAN which would ensure compat with inert="". This also has the benefit to prevent future compat issues like hidden has: adding support for hidden="until-found" (see https://github.com/facebook/react/issues/24740) means hidden="" will now be considered true instead of false.
Comparing: bb0944fe5bdd619be918621a9a1647204d6e7ce1...cf2173e46ce4a9d63694046d9eef1c8ee6cbc8e1
Critical size changes
Includes critical production bundles, as well as any change greater than 2%:
| Name | +/- | Base | Current | +/- gzip | Base gzip | Current gzip |
|---|---|---|---|---|---|---|
| oss-stable/react-dom/cjs/react-dom.production.min.js | +0.02% | 176.80 kB | 176.83 kB | +0.01% | 54.90 kB | 54.91 kB |
| oss-experimental/react-dom/cjs/react-dom.production.min.js | = | 173.53 kB | 173.55 kB | +0.01% | 54.11 kB | 54.11 kB |
| facebook-www/ReactDOM-prod.classic.js | +0.01% | 593.95 kB | 594.04 kB | = | 104.36 kB | 104.37 kB |
| facebook-www/ReactDOM-prod.modern.js | +0.01% | 577.21 kB | 577.30 kB | = | 101.41 kB | 101.42 kB |
| test_utils/ReactAllWarnings.js | Deleted | 66.60 kB | 0.00 kB | Deleted | 16.28 kB | 0.00 kB |
Significant size changes
Includes any change greater than 0.2%:
Expand to show
Generated by :no_entry_sign: dangerJS against cf2173e46ce4a9d63694046d9eef1c8ee6cbc8e1
All major browsers support the inert attribute.
The original request for React to "whitelist" this attribute was opened 3 1/2 years ago, presumably to prevent this very situation where React, not the browsers, is the blocker.
This PR was opened 1 year ago. It remains open and those who maintain React projects are prevented from using this new HTML attribute.
I'm sure the React team is a great bunch of folks - honestly - but it is unacceptable for a project like React to drag its feet for more than 3 years now and prevent developers from using native HTML features. This is IE behavior. React isn't a little open-source project maintained by volunteers, React is a product marketed to developers with a multi-million dollar budget and paid full-time engineers. Again, great people I'm sure, but great people don't always deliver early or even on time. Support for inert is objectively very late. Let's please get this merged!
Its 2023, June, and React still doesn't recognize the inert attribute. I really hope its implemented as soon as possible. Such a basic thing taking sooooo long
@jfbrennan @DIPANJAN01 @ingomc sorry about this taking so long. in the mean time you should be able to work around this by using adding the inert="" attribute e.g. <div inert="" />. If this workaround doesn't' work for your use case please let us know so we have the chance to fix it in this PR.
Thanks @mattcarrollcode I did just that and also had to add this to get TS to shut up:
declare module 'react' {
interface HTMLAttributes<T> extends AriaAttributes, DOMAttributes<T> {
inert?: ''; // TODO Treating this totally valid HTML attribute as custom to make React work. Remove after this bug is fixed https://github.com/facebook/react/pull/24730
}
}
