Skip to content

Commit

Permalink
Land enableNewBooleanProps everywhere (#28676)
Browse files Browse the repository at this point in the history
Rolled out internally. Removing flag.
  • Loading branch information
jackpope committed Mar 29, 2024
1 parent 425f72b commit 6cd6ba7
Show file tree
Hide file tree
Showing 14 changed files with 74 additions and 140 deletions.
75 changes: 34 additions & 41 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ import {
disableIEWorkarounds,
enableTrustedTypesIntegration,
enableFilterEmptyStringAttributesDOM,
enableNewBooleanProps,
} from 'shared/ReactFeatureFlags';
import {
mediaEventTypes,
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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 (
Expand Down
39 changes: 18 additions & 21 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
enableBigIntSupport,
enableFilterEmptyStringAttributesDOM,
enableFizzExternalRuntime,
enableNewBooleanProps,
} from 'shared/ReactFeatureFlags';

import type {
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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./;
Expand Down Expand Up @@ -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-') {
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -83,6 +82,7 @@ const possibleStandardNames = {
id: 'id',
imagesizes: 'imageSizes',
imagesrcset: 'imageSrcSet',
inert: 'inert',
innerhtml: 'innerHTML',
inputmode: 'inputMode',
integrity: 'integrity',
Expand Down Expand Up @@ -503,8 +503,4 @@ const possibleStandardNames = {
zoomandpan: 'zoomAndPan',
};

if (enableNewBooleanProps) {
possibleStandardNames.inert = 'inert';
}

export default possibleStandardNames;
34 changes: 10 additions & 24 deletions packages/react-dom/src/__tests__/ReactDOMAttribute-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down Expand Up @@ -98,15 +96,9 @@ describe('ReactDOM unknown attribute', () => {
await act(() => {
root.render(<div inert={true} />);
});
}).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 () => {
Expand All @@ -117,20 +109,14 @@ describe('ReactDOM unknown attribute', () => {
await act(() => {
root.render(<div inert="" />);
});
}).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(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,36 +742,24 @@ describe('ReactDOMServerIntegration', () => {
});

itRenders('new boolean `true` attributes', async render => {
const element = await render(
<div inert={true} />,
ReactFeatureFlags.enableNewBooleanProps ? 0 : 1,
);
const element = await render(<div inert={true} />, 0);

expect(element.getAttribute('inert')).toBe(
ReactFeatureFlags.enableNewBooleanProps ? '' : null,
);
expect(element.getAttribute('inert')).toBe('');
});

itRenders('new boolean `""` attributes', async render => {
const element = await render(
<div inert="" />,
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(
<div inert={false} />,
ReactFeatureFlags.enableNewBooleanProps ? 0 : 1,
);
const element = await render(<div inert={false} />, 0);

expect(element.getAttribute('inert')).toBe(null);
});
Expand Down
7 changes: 0 additions & 7 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<div someBooleanAttribute="" />`.
// 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 <Context> equivalent to <Context.Provider> instead of <Context.Consumer>
export const enableRenderableContext = __NEXT_MAJOR__;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 6cd6ba7

Please sign in to comment.