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

React DOM: Support boolean values for inert prop #24730

Merged
merged 11 commits into from
Mar 13, 2024
14 changes: 7 additions & 7 deletions fixtures/attribute-behavior/AttributeTableSnapshot.md
Original file line number Diff line number Diff line change
Expand Up @@ -5427,20 +5427,20 @@
| Test Case | Flags | Result |
| --- | --- | --- |
| `inert=(string)`| (changed)| `<boolean: true>` |
| `inert=(empty string)`| (changed)| `<boolean: true>` |
| `inert=(empty string)`| (initial, warning)| `<boolean: false>` |
| `inert=(array with string)`| (changed)| `<boolean: true>` |
| `inert=(empty array)`| (changed)| `<boolean: true>` |
| `inert=(object)`| (changed)| `<boolean: true>` |
| `inert=(numeric string)`| (changed)| `<boolean: true>` |
| `inert=(-1)`| (changed)| `<boolean: true>` |
| `inert=(0)`| (changed)| `<boolean: true>` |
| `inert=(0)`| (initial)| `<boolean: false>` |
| `inert=(integer)`| (changed)| `<boolean: true>` |
| `inert=(NaN)`| (changed, warning)| `<boolean: true>` |
| `inert=(NaN)`| (initial, warning)| `<boolean: false>` |
| `inert=(float)`| (changed)| `<boolean: true>` |
| `inert=(true)`| (initial, warning)| `<boolean: false>` |
| `inert=(false)`| (initial, warning)| `<boolean: false>` |
| `inert=(string 'true')`| (changed)| `<boolean: true>` |
| `inert=(string 'false')`| (changed)| `<boolean: true>` |
| `inert=(true)`| (changed)| `<boolean: true>` |
| `inert=(false)`| (initial)| `<boolean: false>` |
| `inert=(string 'true')`| (changed, warning)| `<boolean: true>` |
| `inert=(string 'false')`| (changed, warning)| `<boolean: true>` |
| `inert=(string 'on')`| (changed)| `<boolean: true>` |
| `inert=(string 'off')`| (changed)| `<boolean: true>` |
| `inert=(symbol)`| (initial, warning)| `<boolean: false>` |
Expand Down
49 changes: 49 additions & 0 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
disableIEWorkarounds,
enableTrustedTypesIntegration,
enableFilterEmptyStringAttributesDOM,
enableNewBooleanProps,
} from 'shared/ReactFeatureFlags';
import {
mediaEventTypes,
Expand All @@ -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
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
enableFloat,
enableFormActions,
enableFizzExternalRuntime,
enableNewBooleanProps,
} from 'shared/ReactFeatureFlags';

import type {
Expand Down Expand Up @@ -345,6 +346,11 @@ const importMapScriptEnd = stringToPrecomputedChunk('</script>');
// 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)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import hasOwnProperty from 'shared/hasOwnProperty';
import {
enableCustomElementPropertySupport,
enableFormActions,
enableNewBooleanProps,
} from 'shared/ReactFeatureFlags';

const warnedProperties = {};
Expand Down Expand Up @@ -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-') {
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -502,4 +503,8 @@ const possibleStandardNames = {
zoomandpan: 'zoomAndPan',
};

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

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

Expand Down Expand Up @@ -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(<div inert={true} />);
});
}).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(<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 : '',
);

// The warning is only printed once.
await act(() => {
root.render(<div inert="" />);
});
});

it('passes through strings', async () => {
await testUnknownAttributeAssignment('a string', 'a string');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,41 @@ describe('ReactDOMServerIntegration', () => {
}
});

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

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

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,
);

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

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

expect(element.getAttribute('inert')).toBe(null);
});

itRenders(
'no unknown attributes for custom elements with null value',
async render => {
Expand Down
7 changes: 7 additions & 0 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<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__;

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

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

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

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

Expand Down
2 changes: 2 additions & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ export const allowConcurrentByDefault = true;

export const consoleManagedByDevToolsDuringStrictMode = true;

export const enableNewBooleanProps = false;

export const enableFizzExternalRuntime = true;

export const forceConcurrentByDefaultForTesting = false;
Expand Down