Match Preact behavior for boolean props on custom elements (#24541)
* Log unexpected warnings when testing with ReactDOMServerIntegrationTestUtils * Add test Following https://github.com/facebook/react/issues/9230#issuecomment-322007671 except that `foo={true}` renders an empty string. See https://github.com/facebook/react/issues/9230#issuecomment-1123464720 for rationale. * Match Preact behavior for boolean props on custom elements * Poke CircleCI
Sebastian Silbermann committed
May 20, 2022 at 19:10 UTC
82c64e1a49239158c0daa7f0d603d2ad2ee667a9
8 files changed
+79
-11
packages/react-dom/src/__tests__/DOMPropertyOperations-test.js
+25
-3
@@ -10,7 +10,10 @@
10
'use strict';
11
12
// Set by `yarn test-fire`.
13
-const {disableInputAttributeSyncing} = require('shared/ReactFeatureFlags');
13
+const {
14
+ enableCustomElementPropertySupport,
15
+ disableInputAttributeSyncing,
16
+} = require('shared/ReactFeatureFlags');
17
18
describe('DOMPropertyOperations', () => {
19
let React;
@@ -256,8 +259,12 @@ describe('DOMPropertyOperations', () => {
259
expect(customElement.getAttribute('onstring')).toBe('hello');
260
expect(customElement.getAttribute('onobj')).toBe('[object Object]');
261
expect(customElement.getAttribute('onarray')).toBe('one,two');
259
- expect(customElement.getAttribute('ontrue')).toBe('true');
260
- expect(customElement.getAttribute('onfalse')).toBe('false');
262
+ expect(customElement.getAttribute('ontrue')).toBe(
263
+ enableCustomElementPropertySupport ? '' : 'true',
264
+ );
265
+ expect(customElement.getAttribute('onfalse')).toBe(
266
+ enableCustomElementPropertySupport ? null : 'false',
267
+ );
268
269
// Dispatch the corresponding event names to make sure that nothing crashes.
270
customElement.dispatchEvent(new Event('string'));
@@ -959,6 +966,21 @@ describe('DOMPropertyOperations', () => {
966
expect(customElement.foo).toBe(null);
967
});
968
969
+ // @gate enableCustomElementPropertySupport
970
+ it('boolean props should not be stringified in attributes', () => {
971
+ const container = document.createElement('div');
972
+ document.body.appendChild(container);
973
+ ReactDOM.render(<my-custom-element foo={true} />, container);
974
+ const customElement = container.querySelector('my-custom-element');
975
+
976
+ expect(customElement.getAttribute('foo')).toBe('');
977
+
978
+ // true => false
979
+ ReactDOM.render(<my-custom-element foo={false} />, container);
980
+
981
+ expect(customElement.getAttribute('foo')).toBe(null);
982
+ });
983
+
984
// @gate enableCustomElementPropertySupport
985
it('custom element custom event handlers assign multiple types', () => {
986
const container = document.createElement('div');
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+6
-2
@@ -2689,9 +2689,13 @@ describe('ReactDOMComponent', () => {
2689
const container = document.createElement('div');
2690
ReactDOM.render(<some-custom-element foo={true} />, container);
2691
const node = container.firstChild;
2692
- expect(node.getAttribute('foo')).toBe('true');
2692
+ expect(node.getAttribute('foo')).toBe(
2693
+ ReactFeatureFlags.enableCustomElementPropertySupport ? '' : 'true',
2694
+ );
2695
ReactDOM.render(<some-custom-element foo={false} />, container);
2694
- expect(node.getAttribute('foo')).toBe('false');
2696
+ expect(node.getAttribute('foo')).toBe(
2697
+ ReactFeatureFlags.enableCustomElementPropertySupport ? null : 'false',
2698
+ );
2699
ReactDOM.render(<some-custom-element />, container);
2700
expect(node.hasAttribute('foo')).toBe(false);
2701
ReactDOM.render(<some-custom-element foo={true} />, container);
packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
+10
-2
@@ -696,12 +696,20 @@ describe('ReactDOMServerIntegration', () => {
696
697
itRenders('unknown boolean `true` attributes as strings', async render => {
698
const e = await render(<custom-element foo={true} />);
699
- expect(e.getAttribute('foo')).toBe('true');
699
+ if (ReactFeatureFlags.enableCustomElementPropertySupport) {
700
+ expect(e.getAttribute('foo')).toBe('');
701
+ } else {
702
+ expect(e.getAttribute('foo')).toBe('true');
703
+ }
704
});
705
706
itRenders('unknown boolean `false` attributes as strings', async render => {
707
const e = await render(<custom-element foo={false} />);
704
- expect(e.getAttribute('foo')).toBe('false');
708
+ if (ReactFeatureFlags.enableCustomElementPropertySupport) {
709
+ expect(e.getAttribute('foo')).toBe(null);
710
+ } else {
711
+ expect(e.getAttribute('foo')).toBe('false');
712
+ }
713
});
714
715
itRenders(
packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js
+1
-1
@@ -89,7 +89,7 @@ module.exports = function(initModules) {
89
console.log(
90
`We expected ${count} warning(s), but saw ${filteredWarnings.length} warning(s).`,
91
);
92
- if (filteredWarnings.count > 0) {
92
+ if (filteredWarnings.length > 0) {
93
console.log(`We saw these warnings:`);
94
for (let i = 0; i < filteredWarnings.length; i++) {
95
console.log(...filteredWarnings[i]);
packages/react-dom/src/client/DOMPropertyOperations.js
+13
@@ -115,6 +115,7 @@ export function getValueForAttribute(
115
node: Element,
116
name: string,
117
expected: mixed,
118
+ isCustomComponentTag: boolean,
119
): mixed {
120
if (__DEV__) {
121
if (!isAttributeNameSafe(name)) {
@@ -124,6 +125,13 @@ export function getValueForAttribute(
125
return expected === undefined ? undefined : null;
126
}
127
const value = node.getAttribute(name);
128
+
129
+ if (enableCustomElementPropertySupport) {
130
+ if (isCustomComponentTag && value === '') {
131
+ return true;
132
+ }
133
+ }
134
+
135
if (__DEV__) {
136
checkAttributeStringCoercion(expected, name);
137
}
@@ -196,6 +204,11 @@ export function setValueForProperty(
204
if (shouldRemoveAttribute(name, value, propertyInfo, isCustomComponentTag)) {
205
value = null;
206
}
207
+ if (enableCustomElementPropertySupport) {
208
+ if (isCustomComponentTag && value === true) {
209
+ value = '';
210
+ }
211
+ }
212
213
// If the prop isn't in the special list, treat it as a simple attribute.
214
if (isCustomComponentTag || propertyInfo === null) {
packages/react-dom/src/client/ReactDOMComponent.js
+12
-2
@@ -1081,7 +1081,12 @@ export function diffHydratedProperties(
1081
} else if (isCustomComponentTag && !enableCustomElementPropertySupport) {
1082
// $FlowFixMe - Should be inferred as not undefined.
1083
extraAttributeNames.delete(propKey.toLowerCase());
1084
- serverValue = getValueForAttribute(domElement, propKey, nextProp);
1084
+ serverValue = getValueForAttribute(
1085
+ domElement,
1086
+ propKey,
1087
+ nextProp,
1088
+ isCustomComponentTag,
1089
+ );
1090
1091
if (nextProp !== serverValue) {
1092
warnForPropDifference(propKey, serverValue, nextProp);
@@ -1128,7 +1133,12 @@ export function diffHydratedProperties(
1133
// $FlowFixMe - Should be inferred as not undefined.
1134
extraAttributeNames.delete(propKey);
1135
}
1131
- serverValue = getValueForAttribute(domElement, propKey, nextProp);
1136
+ serverValue = getValueForAttribute(
1137
+ domElement,
1138
+ propKey,
1139
+ nextProp,
1140
+ isCustomComponentTag,
1141
+ );
1142
}
1143
1144
const dontWarnCustomElement =
packages/react-dom/src/server/ReactDOMServerFormatConfig.js
+7
-1
@@ -1156,7 +1156,7 @@ function pushStartCustomElement(
1156
let innerHTML = null;
1157
for (let propKey in props) {
1158
if (hasOwnProperty.call(props, propKey)) {
1159
- const propValue = props[propKey];
1159
+ let propValue = props[propKey];
1160
if (propValue == null) {
1161
continue;
1162
}
@@ -1169,6 +1169,12 @@ function pushStartCustomElement(
1169
// so skip it.
1170
continue;
1171
}
1172
+ if (enableCustomElementPropertySupport && propValue === false) {
1173
+ continue;
1174
+ }
1175
+ if (enableCustomElementPropertySupport && propValue === true) {
1176
+ propValue = '';
1177
+ }
1178
if (enableCustomElementPropertySupport && propKey === 'className') {
1179
// className gets rendered as class on the client, so it should be
1180
// rendered as class on the server.
packages/react-dom/src/shared/DOMProperty.js
+5
@@ -162,6 +162,11 @@ export function shouldRemoveAttribute(
162
return true;
163
}
164
if (isCustomComponentTag) {
165
+ if (enableCustomElementPropertySupport) {
166
+ if (value === false) {
167
+ return true;
168
+ }
169
+ }
170
return false;
171
}
172
if (propertyInfo !== null) {