From e5ab2944f9b27379fc52a4464159f34c7618d6ef Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Wed, 13 Mar 2024 23:10:41 +0100 Subject: [PATCH] React DOM: Support boolean values for `inert` prop (#24730) --- .../AttributeTableSnapshot.md | 14 +++--- .../src/client/ReactDOMComponent.js | 49 ++++++++++++++++++ .../src/server/ReactFizzConfigDOM.js | 32 ++++++++++++ .../src/shared/ReactDOMUnknownPropertyHook.js | 15 ++++++ .../src/shared/possibleStandardNames.js | 5 ++ .../src/__tests__/ReactDOMAttribute-test.js | 50 +++++++++++++++++++ ...eactDOMServerIntegrationAttributes-test.js | 35 +++++++++++++ packages/shared/ReactFeatureFlags.js | 7 +++ .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 2 + 14 files changed, 207 insertions(+), 7 deletions(-) diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md index f3a8ebfde9923..8db00f818fbb3 100644 --- a/fixtures/attribute-behavior/AttributeTableSnapshot.md +++ b/fixtures/attribute-behavior/AttributeTableSnapshot.md @@ -5427,20 +5427,20 @@ | Test Case | Flags | Result | | --- | --- | --- | | `inert=(string)`| (changed)| `` | -| `inert=(empty string)`| (changed)| `` | +| `inert=(empty string)`| (initial, warning)| `` | | `inert=(array with string)`| (changed)| `` | | `inert=(empty array)`| (changed)| `` | | `inert=(object)`| (changed)| `` | | `inert=(numeric string)`| (changed)| `` | | `inert=(-1)`| (changed)| `` | -| `inert=(0)`| (changed)| `` | +| `inert=(0)`| (initial)| `` | | `inert=(integer)`| (changed)| `` | -| `inert=(NaN)`| (changed, warning)| `` | +| `inert=(NaN)`| (initial, warning)| `` | | `inert=(float)`| (changed)| `` | -| `inert=(true)`| (initial, warning)| `` | -| `inert=(false)`| (initial, warning)| `` | -| `inert=(string 'true')`| (changed)| `` | -| `inert=(string 'false')`| (changed)| `` | +| `inert=(true)`| (changed)| `` | +| `inert=(false)`| (initial)| `` | +| `inert=(string 'true')`| (changed, warning)| `` | +| `inert=(string 'false')`| (changed, warning)| `` | | `inert=(string 'on')`| (changed)| `` | | `inert=(string 'off')`| (changed)| `` | | `inert=(symbol)`| (initial, warning)| `` | diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index d54573bdffbb1..bd071b4fd17bc 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -73,6 +73,7 @@ import { disableIEWorkarounds, enableTrustedTypesIntegration, enableFilterEmptyStringAttributesDOM, + enableNewBooleanProps, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -86,8 +87,10 @@ let didWarnFormActionType = false; let didWarnFormActionName = false; let didWarnFormActionTarget = false; let didWarnFormActionMethod = false; +let didWarnForNewBooleanPropsWithEmptyValue: {[string]: boolean}; let canDiffStyleForHydrationWarning; if (__DEV__) { + didWarnForNewBooleanPropsWithEmptyValue = {}; // IE 11 parses & normalizes the style attribute as opposed to other // browsers. It adds spaces and sorts the properties in some // non-alphabetical order. Handling that would require sorting CSS @@ -712,6 +715,25 @@ function setProp( break; } // Boolean + case 'inert': + if (!enableNewBooleanProps) { + setValueForAttribute(domElement, key, value); + break; + } else { + if (__DEV__) { + if (value === '' && !didWarnForNewBooleanPropsWithEmptyValue[key]) { + didWarnForNewBooleanPropsWithEmptyValue[key] = true; + console.error( + 'Received an empty string for a boolean attribute `%s`. ' + + 'This will treat the attribute as if it were false. ' + + 'Either pass `false` to silence this warning, or ' + + 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.', + key, + ); + } + } + } + // fallthrough for new boolean props without the flag on case 'allowFullScreen': case 'async': case 'autoPlay': @@ -2663,6 +2685,33 @@ function diffHydratedGenericElement( extraAttributes, ); continue; + case 'inert': + if (enableNewBooleanProps) { + if (__DEV__) { + if ( + value === '' && + !didWarnForNewBooleanPropsWithEmptyValue[propKey] + ) { + didWarnForNewBooleanPropsWithEmptyValue[propKey] = true; + console.error( + 'Received an empty string for a boolean attribute `%s`. ' + + 'This will treat the attribute as if it were false. ' + + 'Either pass `false` to silence this warning, or ' + + 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.', + propKey, + ); + } + } + hydrateBooleanAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); + continue; + } + // fallthrough for new boolean props without the flag on default: { if ( // shouldIgnoreAttribute diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 5902ece4d718e..8b45146beabdf 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -34,6 +34,7 @@ import { enableFloat, enableFormActions, enableFizzExternalRuntime, + enableNewBooleanProps, } from 'shared/ReactFeatureFlags'; import type { @@ -345,6 +346,11 @@ const importMapScriptEnd = stringToPrecomputedChunk(''); // allow one more header to be captured which means in practice if the limit is approached it will be exceeded const DEFAULT_HEADERS_CAPACITY_IN_UTF16_CODE_UNITS = 2000; +let didWarnForNewBooleanPropsWithEmptyValue: {[string]: boolean}; +if (__DEV__) { + didWarnForNewBooleanPropsWithEmptyValue = {}; +} + // Allows us to keep track of what we've already written so we can refer back to it. // if passed externalRuntimeConfig and the enableFizzExternalRuntime feature flag // is set, the server will send instructions via data attributes (instead of inline scripts) @@ -1398,6 +1404,32 @@ function pushAttribute( case 'xmlSpace': pushStringAttribute(target, 'xml:space', value); return; + case 'inert': { + if (enableNewBooleanProps) { + if (__DEV__) { + if (value === '' && !didWarnForNewBooleanPropsWithEmptyValue[name]) { + didWarnForNewBooleanPropsWithEmptyValue[name] = true; + console.error( + 'Received an empty string for a boolean attribute `%s`. ' + + 'This will treat the attribute as if it were false. ' + + 'Either pass `false` to silence this warning, or ' + + 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.', + name, + ); + } + } + // Boolean + if (value && typeof value !== 'function' && typeof value !== 'symbol') { + target.push( + attributeSeparator, + stringToChunk(name), + attributeEmptyString, + ); + } + return; + } + } + // fallthrough for new boolean props without the flag on default: if ( // shouldIgnoreAttribute diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index 0648059695ecb..29767d8a65ffb 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -12,6 +12,7 @@ import hasOwnProperty from 'shared/hasOwnProperty'; import { enableCustomElementPropertySupport, enableFormActions, + enableNewBooleanProps, } from 'shared/ReactFeatureFlags'; const warnedProperties = {}; @@ -240,6 +241,14 @@ function validateProperty(tagName, name, value, eventRegistry) { // Boolean properties can accept boolean values return true; } + // fallthrough + case 'inert': { + if (enableNewBooleanProps) { + // Boolean properties can accept boolean values + return true; + } + } + // fallthrough for new boolean props without the flag on default: { const prefix = name.toLowerCase().slice(0, 5); if (prefix === 'data-' || prefix === 'aria-') { @@ -314,6 +323,12 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'itemScope': { break; } + case 'inert': { + if (enableNewBooleanProps) { + break; + } + } + // fallthrough for new boolean props without the flag on default: { return true; } diff --git a/packages/react-dom-bindings/src/shared/possibleStandardNames.js b/packages/react-dom-bindings/src/shared/possibleStandardNames.js index bd093192a3d32..d2d90877b53d7 100644 --- a/packages/react-dom-bindings/src/shared/possibleStandardNames.js +++ b/packages/react-dom-bindings/src/shared/possibleStandardNames.js @@ -4,6 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. */ +import {enableNewBooleanProps} from 'shared/ReactFeatureFlags'; // When adding attributes to the HTML or SVG allowed attribute list, be sure to // also add them to this module to ensure casing and incorrect name @@ -502,4 +503,8 @@ const possibleStandardNames = { zoomandpan: 'zoomAndPan', }; +if (enableNewBooleanProps) { + possibleStandardNames.inert = 'inert'; +} + export default possibleStandardNames; diff --git a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js index f6e82f25a5e8a..402693c407d5c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js @@ -12,12 +12,14 @@ describe('ReactDOM unknown attribute', () => { let React; let ReactDOMClient; + let ReactFeatureFlags; let act; beforeEach(() => { jest.resetModules(); React = require('react'); ReactDOMClient = require('react-dom/client'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); act = require('internal-test-utils').act; }); @@ -88,6 +90,54 @@ describe('ReactDOM unknown attribute', () => { expect(el.firstChild.hasAttribute('unknown')).toBe(false); }); + it('removes new boolean props', async () => { + const el = document.createElement('div'); + const root = ReactDOMClient.createRoot(el); + + await expect(async () => { + await act(() => { + root.render(
); + }); + }).toErrorDev( + ReactFeatureFlags.enableNewBooleanProps + ? [] + : ['Warning: Received `true` for a non-boolean attribute `inert`.'], + ); + + expect(el.firstChild.getAttribute('inert')).toBe( + ReactFeatureFlags.enableNewBooleanProps ? '' : null, + ); + }); + + it('warns once for empty strings in new boolean props', async () => { + const el = document.createElement('div'); + const root = ReactDOMClient.createRoot(el); + + await expect(async () => { + await act(() => { + root.render(
); + }); + }).toErrorDev( + ReactFeatureFlags.enableNewBooleanProps + ? [ + 'Warning: Received an empty string for a boolean attribute `inert`. ' + + 'This will treat the attribute as if it were false. ' + + 'Either pass `false` to silence this warning, or ' + + 'pass `true` if you used an empty string in earlier versions of React to indicate this attribute is true.', + ] + : [], + ); + + expect(el.firstChild.getAttribute('inert')).toBe( + ReactFeatureFlags.enableNewBooleanProps ? null : '', + ); + + // The warning is only printed once. + await act(() => { + root.render(
); + }); + }); + it('passes through strings', async () => { await testUnknownAttributeAssignment('a string', 'a string'); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js index 0880f2c6b0557..1613ef66994ef 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js @@ -754,6 +754,41 @@ describe('ReactDOMServerIntegration', () => { } }); + itRenders('new boolean `true` attributes', async render => { + const element = await render( +
, + ReactFeatureFlags.enableNewBooleanProps ? 0 : 1, + ); + + expect(element.getAttribute('inert')).toBe( + ReactFeatureFlags.enableNewBooleanProps ? '' : null, + ); + }); + + itRenders('new boolean `""` attributes', async render => { + const element = await render( +
, + ReactFeatureFlags.enableNewBooleanProps + ? // Warns since this used to render `inert=""` like `inert={true}` + // but now renders it like `inert={false}`. + 1 + : 0, + ); + + expect(element.getAttribute('inert')).toBe( + ReactFeatureFlags.enableNewBooleanProps ? null : '', + ); + }); + + itRenders('new boolean `false` attributes', async render => { + const element = await render( +
, + ReactFeatureFlags.enableNewBooleanProps ? 0 : 1, + ); + + expect(element.getAttribute('inert')).toBe(null); + }); + itRenders( 'no unknown attributes for custom elements with null value', async render => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3326b3bdc74a0..ff03d8849afe2 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -198,6 +198,13 @@ export const enableReactTestRendererWarning = false; // before removing them in stable in the next Major export const disableLegacyMode = __NEXT_MAJOR__; +// HTML boolean attributes need a special PropertyInfoRecord. +// Between support of these attributes in browsers and React supporting them as +// boolean props library users can use them as `
`. +// However, once React considers them as boolean props an empty string will +// result in false property i.e. break existing usage. +export const enableNewBooleanProps = __NEXT_MAJOR__; + // ----------------------------------------------------------------------------- // Chopping Block // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f56ad6f00eb83..93a8d66ce7da3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -80,6 +80,7 @@ export const enableLegacyHidden = false; export const forceConcurrentByDefaultForTesting = false; export const allowConcurrentByDefault = false; export const enableCustomElementPropertySupport = true; +export const enableNewBooleanProps = true; export const enableTransitionTracing = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index b8da8afffbe28..acc051edcc661 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -63,6 +63,7 @@ export const forceConcurrentByDefaultForTesting = false; export const enableUnifiedSyncLane = true; export const allowConcurrentByDefault = false; export const enableCustomElementPropertySupport = true; +export const enableNewBooleanProps = true; export const consoleManagedByDevToolsDuringStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 0905df2955a05..366e475a4f57a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -99,6 +99,7 @@ export const enableReactTestRendererWarning = false; export const enableBigIntSupport = __NEXT_MAJOR__; export const disableLegacyMode = __NEXT_MAJOR__; export const disableLegacyContext = __NEXT_MAJOR__; +export const enableNewBooleanProps = __NEXT_MAJOR__; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index f7673e51cfd0a..fd9531385971b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -63,6 +63,7 @@ export const forceConcurrentByDefaultForTesting = false; export const enableUnifiedSyncLane = true; export const allowConcurrentByDefault = true; export const enableCustomElementPropertySupport = true; +export const enableNewBooleanProps = true; export const consoleManagedByDevToolsDuringStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index d8b43417dd09d..f6b6be8054609 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -63,6 +63,7 @@ export const forceConcurrentByDefaultForTesting = false; export const enableUnifiedSyncLane = true; export const allowConcurrentByDefault = true; export const enableCustomElementPropertySupport = false; +export const enableNewBooleanProps = false; export const consoleManagedByDevToolsDuringStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index b51aa5b7ec188..d2364057ecadf 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -102,6 +102,8 @@ export const allowConcurrentByDefault = true; export const consoleManagedByDevToolsDuringStrictMode = true; +export const enableNewBooleanProps = false; + export const enableFizzExternalRuntime = true; export const forceConcurrentByDefaultForTesting = false;