From 6cd6ba703de77e332ab201518b6e30e47cd49aaf Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Fri, 29 Mar 2024 16:02:32 -0400 Subject: [PATCH] Land enableNewBooleanProps everywhere (#28676) Rolled out internally. Removing flag. --- .../src/client/ReactDOMComponent.js | 75 +++++++++---------- .../src/server/ReactFizzConfigDOM.js | 39 +++++----- .../src/shared/ReactDOMUnknownPropertyHook.js | 20 +---- .../src/shared/possibleStandardNames.js | 6 +- .../src/__tests__/ReactDOMAttribute-test.js | 34 +++------ ...eactDOMServerIntegrationAttributes-test.js | 26 ++----- 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 - .../forks/ReactFeatureFlags.www-dynamic.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - 14 files changed, 74 insertions(+), 140 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 523babf87eed2..715b241cdb02f 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -70,7 +70,6 @@ import { disableIEWorkarounds, enableTrustedTypesIntegration, enableFilterEmptyStringAttributesDOM, - enableNewBooleanProps, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -668,24 +667,20 @@ 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, - ); - } + case 'inert': { + 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': @@ -2764,32 +2759,30 @@ function diffHydratedGenericElement( ); 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, - ); - } + 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, - serverDifferences, - ); - continue; } + hydrateBooleanAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + serverDifferences, + ); + continue; // fallthrough for new boolean props without the flag on default: { if ( diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index 6716811bf486a..423e6aba6c839 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -31,7 +31,6 @@ import { enableBigIntSupport, enableFilterEmptyStringAttributesDOM, enableFizzExternalRuntime, - enableNewBooleanProps, } from 'shared/ReactFeatureFlags'; import type { @@ -1423,29 +1422,27 @@ function pushAttribute( 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, + 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, ); } - return; } + // 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: diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index b80d23cbd0b84..3e49fa637f29d 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -9,7 +9,6 @@ import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe'; import isCustomElement from './isCustomElement'; import possibleStandardNames from './possibleStandardNames'; import hasOwnProperty from 'shared/hasOwnProperty'; -import {enableNewBooleanProps} from 'shared/ReactFeatureFlags'; const warnedProperties = {}; const EVENT_NAME_REGEX = /^on./; @@ -228,18 +227,12 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'seamless': case 'itemScope': case 'capture': - case 'download': { + case 'download': + case 'inert': { // 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-') { @@ -311,15 +304,10 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'reversed': case 'scoped': case 'seamless': - case 'itemScope': { - break; - } + case 'itemScope': case 'inert': { - if (enableNewBooleanProps) { - break; - } + 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 d2d90877b53d7..6c06b47c64ac7 100644 --- a/packages/react-dom-bindings/src/shared/possibleStandardNames.js +++ b/packages/react-dom-bindings/src/shared/possibleStandardNames.js @@ -4,7 +4,6 @@ * 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 @@ -83,6 +82,7 @@ const possibleStandardNames = { id: 'id', imagesizes: 'imageSizes', imagesrcset: 'imageSrcSet', + inert: 'inert', innerhtml: 'innerHTML', inputmode: 'inputMode', integrity: 'integrity', @@ -503,8 +503,4 @@ 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 402693c407d5c..13fa1cba4993d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js @@ -12,14 +12,12 @@ 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; }); @@ -98,15 +96,9 @@ describe('ReactDOM unknown attribute', () => { await act(() => { root.render(
); }); - }).toErrorDev( - ReactFeatureFlags.enableNewBooleanProps - ? [] - : ['Warning: Received `true` for a non-boolean attribute `inert`.'], - ); + }).toErrorDev([]); - expect(el.firstChild.getAttribute('inert')).toBe( - ReactFeatureFlags.enableNewBooleanProps ? '' : null, - ); + expect(el.firstChild.getAttribute('inert')).toBe(true ? '' : null); }); it('warns once for empty strings in new boolean props', async () => { @@ -117,20 +109,14 @@ describe('ReactDOM unknown attribute', () => { 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 : '', - ); + }).toErrorDev([ + '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(true ? null : ''); // The warning is only printed once. await act(() => { diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js index 80467e5efc5b1..5d798e3f58df3 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js @@ -742,36 +742,24 @@ describe('ReactDOMServerIntegration', () => { }); itRenders('new boolean `true` attributes', async render => { - const element = await render( -
, - ReactFeatureFlags.enableNewBooleanProps ? 0 : 1, - ); + const element = await render(
, 0); - expect(element.getAttribute('inert')).toBe( - ReactFeatureFlags.enableNewBooleanProps ? '' : null, - ); + expect(element.getAttribute('inert')).toBe(''); }); 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, + // Warns since this used to render `inert=""` like `inert={true}` + // but now renders it like `inert={false}`. + 1, ); - expect(element.getAttribute('inert')).toBe( - ReactFeatureFlags.enableNewBooleanProps ? null : '', - ); + expect(element.getAttribute('inert')).toBe(null); }); itRenders('new boolean `false` attributes', async render => { - const element = await render( -
, - ReactFeatureFlags.enableNewBooleanProps ? 0 : 1, - ); + const element = await render(
, 0); expect(element.getAttribute('inert')).toBe(null); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index d2fad7905613d..4d4fc361f4879 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -183,13 +183,6 @@ export const disableLegacyMode = __NEXT_MAJOR__; export const disableDOMTestUtils = __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__; - // Make equivalent to instead of export const enableRenderableContext = __NEXT_MAJOR__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index cf821b1554f8e..13e8d00c5b3b3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -78,7 +78,6 @@ export const enableLazyContextPropagation = false; export const enableLegacyHidden = false; export const forceConcurrentByDefaultForTesting = false; export const allowConcurrentByDefault = false; -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 710b7ed4efffe..904f165c214cb 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -99,7 +99,6 @@ export const enableLazyContextPropagation = false; export const enableLegacyHidden = false; export const forceConcurrentByDefaultForTesting = false; export const allowConcurrentByDefault = false; -export const enableNewBooleanProps = true; export const enableTransitionTracing = false; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const passChildrenWhenCloningPersistedNodes = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6d6ab23ae11b3..df84dd4d4cc60 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -92,7 +92,6 @@ export const enableBigIntSupport = __NEXT_MAJOR__; export const disableLegacyMode = __NEXT_MAJOR__; export const disableLegacyContext = __NEXT_MAJOR__; export const disableDOMTestUtils = __NEXT_MAJOR__; -export const enableNewBooleanProps = __NEXT_MAJOR__; export const enableRenderableContext = __NEXT_MAJOR__; export const enableReactTestRendererWarning = __NEXT_MAJOR__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index d36d9cfb60f69..879806d751575 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -59,7 +59,6 @@ export const enableLegacyHidden = false; export const forceConcurrentByDefaultForTesting = false; export const enableUnifiedSyncLane = true; export const allowConcurrentByDefault = 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 671d55aa82fa9..137852fad7c5c 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -59,7 +59,6 @@ export const enableLegacyHidden = false; export const forceConcurrentByDefaultForTesting = false; export const enableUnifiedSyncLane = true; export const allowConcurrentByDefault = true; -export const enableNewBooleanProps = false; export const consoleManagedByDevToolsDuringStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index b542bbb10a1ab..95b9927092fc6 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -30,7 +30,6 @@ export const enableUseDeferredValueInitialArg = __VARIANT__; export const enableRenderableContext = __VARIANT__; export const useModernStrictMode = __VARIANT__; export const enableRefAsProp = __VARIANT__; -export const enableNewBooleanProps = __VARIANT__; export const enableRetryLaneExpiration = __VARIANT__; export const favorSafetyOverHydrationPerf = __VARIANT__; export const retryLaneExpirationMs = 5000; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index fccd09645ed3e..79ff994a0ad2d 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -38,7 +38,6 @@ export const { enableRenderableContext, useModernStrictMode, enableRefAsProp, - enableNewBooleanProps, favorSafetyOverHydrationPerf, } = dynamicFeatureFlags;