Skip to content
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

Land enableNewBooleanProps everywhere #28676

Merged
merged 2 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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