Add warning if you use inert=""

facebook · Feb 21, 2024 · 057f8f3 · 057f8f3
1 parent 7f5f16e
commit 057f8f3
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 3 deletions.
diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md
@@ -5427,7 +5427,7 @@
 | Test Case | Flags | Result |
 | --- | --- | --- |
 | `inert=(string)`| (changed)| `<boolean: true>` |
-| `inert=(empty string)`| (initial)| `<boolean: false>` |
+| `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>` |

diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -86,8 +86,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
@@ -711,6 +713,19 @@ function setProp(
       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':
@@ -2662,6 +2677,21 @@ 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,
+              );
+            }
+          }
           hydrateBooleanAttribute(
             domElement,
             propKey,

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -347,6 +347,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)
@@ -1402,6 +1407,18 @@ function pushAttribute(
       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(

diff --git a/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js b/packages/react-dom/src/__tests__/ReactDOMAttribute-test.js
@@ -109,6 +109,35 @@ describe('ReactDOM unknown attribute', () => {
       );
     });
 
+    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');
     });

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
@@ -754,7 +754,7 @@ describe('ReactDOMServerIntegration', () => {
       }
     });
 
-    itRenders('new boolean `true` attributes as strings', async render => {
+    itRenders('new boolean `true` attributes', async render => {
       const element = await render(
         <div inert={true} />,
         ReactFeatureFlags.enableNewBooleanProps ? 0 : 1,
@@ -765,7 +765,22 @@ describe('ReactDOMServerIntegration', () => {
       );
     });
 
-    itRenders('new boolean `false` attributes as strings', async render => {
+    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,