Skip to content

Commit

Permalink
React DOM: Support boolean values for inert prop (facebook#24730)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon authored and AndyPengc12 committed Apr 15, 2024
1 parent 3b81f25 commit e5ab294
Show file tree
Hide file tree
Showing 14 changed files with 207 additions and 7 deletions.
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

0 comments on commit e5ab294

Please sign in to comment.