[Flight] Don't warn for key, but error for ref (#19986)

sebmarkbage · web-flow · commit 0a4c7c565146 · 2020-10-08T17:02:23.000-07:00
* Improve error message by expanding the object in question

* Don't warn for key/ref getters

* Error if refs are passed in server components or to client components
diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -165,9 +165,15 @@ describe('ReactFlight', () => {
       return <div foo={Symbol('foo')} />;
     }
 
+    const ref = React.createRef();
+    function RefProp() {
+      return <div ref={ref} />;
+    }
+
     const event = ReactNoopFlightServer.render(<EventHandlerProp />);
     const fn = ReactNoopFlightServer.render(<FunctionProp />);
     const symbol = ReactNoopFlightServer.render(<SymbolProp />);
+    const refs = ReactNoopFlightServer.render(<RefProp />);
 
     function Client({transport}) {
       return ReactNoopFlightClient.read(transport);
@@ -185,6 +191,9 @@ describe('ReactFlight', () => {
           <ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
             <Client transport={symbol} />
           </ErrorBoundary>
+          <ErrorBoundary expectedMessage="Refs cannot be used in server components, nor passed to client components.">
+            <Client transport={refs} />
+          </ErrorBoundary>
         </>,
       );
     });
@@ -217,6 +226,13 @@ describe('ReactFlight', () => {
     );
   });
 
+  it('should NOT warn in DEV for key getters', () => {
+    const transport = ReactNoopFlightServer.render(<div key="a" />);
+    act(() => {
+      ReactNoop.render(ReactNoopFlightClient.read(transport));
+    });
+  });
+
   it('should warn in DEV if an object with symbols is passed to a host component', () => {
     expect(() => {
       const transport = ReactNoopFlightServer.render(
diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
@@ -119,6 +119,15 @@ export function createRequest(
 function attemptResolveElement(element: React$Element<any>): ReactModel {
   const type = element.type;
   const props = element.props;
+  if (element.ref !== null && element.ref !== undefined) {
+    // When the ref moves to the regular props object this will implicitly
+    // throw for functions. We could probably relax it to a DEV warning for other
+    // cases.
+    invariant(
+      false,
+      'Refs cannot be used in server components, nor passed to client components.',
+    );
+  }
   if (typeof type === 'function') {
     // This is a server-side component.
     return type(props);
@@ -153,7 +162,11 @@ function attemptResolveElement(element: React$Element<any>): ReactModel {
       }
     }
   }
-  invariant(false, 'Unsupported type.');
+  invariant(
+    false,
+    'Unsupported server component type: %s',
+    describeValueForErrorMessage(type),
+  );
 }
 
 function pingSegment(request: Request, segment: Segment): void {
@@ -218,7 +231,19 @@ function isSimpleObject(object): boolean {
   const names = Object.getOwnPropertyNames(object);
   for (let i = 0; i < names.length; i++) {
     const descriptor = Object.getOwnPropertyDescriptor(object, names[i]);
-    if (!descriptor || !descriptor.enumerable) {
+    if (!descriptor) {
+      return false;
+    }
+    if (!descriptor.enumerable) {
+      if (
+        (names[i] === 'key' || names[i] === 'ref') &&
+        typeof descriptor.get === 'function'
+      ) {
+        // React adds key and ref getters to props objects to issue warnings.
+        // Those getters will not be transferred to the client, but that's ok,
+        // so we'll special case them.
+        continue;
+      }
       return false;
     }
   }
@@ -249,7 +274,7 @@ function describeValueForErrorMessage(value: ReactModel): string {
         return '[...]';
       }
       const name = objectName(value);
-      if (name === '[object Object]') {
+      if (name === 'Object') {
         return '{...}';
       }
       return name;
@@ -266,6 +291,7 @@ function describeObjectForErrorMessage(
   objectOrArray:
     | {+[key: string | number]: ReactModel}
     | $ReadOnlyArray<ReactModel>,
+  expandedName?: string,
 ): string {
   if (isArray(objectOrArray)) {
     let str = '[';
@@ -279,7 +305,16 @@ function describeObjectForErrorMessage(
         str += '...';
         break;
       }
-      str += describeValueForErrorMessage(array[i]);
+      const value = array[i];
+      if (
+        '' + i === expandedName &&
+        typeof value === 'object' &&
+        value !== null
+      ) {
+        str += describeObjectForErrorMessage(value);
+      } else {
+        str += describeValueForErrorMessage(value);
+      }
     }
     str += ']';
     return str;
@@ -297,10 +332,17 @@ function describeObjectForErrorMessage(
         break;
       }
       const name = names[i];
-      str +=
-        describeKeyForErrorMessage(name) +
-        ': ' +
-        describeValueForErrorMessage(object[name]);
+      str += describeKeyForErrorMessage(name) + ': ';
+      const value = object[name];
+      if (
+        name === expandedName &&
+        typeof value === 'object' &&
+        value !== null
+      ) {
+        str += describeObjectForErrorMessage(value);
+      } else {
+        str += describeValueForErrorMessage(value);
+      }
     }
     str += '}';
     return str;
@@ -444,7 +486,7 @@ export function resolveModelToJSON(
               'Classes or other objects with methods are not supported. ' +
               'Remove %s from these props: %s',
             describeKeyForErrorMessage(key),
-            describeObjectForErrorMessage(parent),
+            describeObjectForErrorMessage(parent, key),
           );
         } else if (Object.getOwnPropertySymbols) {
           const symbols = Object.getOwnPropertySymbols(value);
@@ -455,7 +497,7 @@ export function resolveModelToJSON(
                 'Remove %s from these props: %s',
               symbols[0].description,
               describeKeyForErrorMessage(key),
-              describeObjectForErrorMessage(parent),
+              describeObjectForErrorMessage(parent, key),
             );
           }
         }
diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
@@ -340,7 +340,7 @@
   "348": "ensureListeningTo(): received a container that was not an element node. This is likely a bug in React.",
   "349": "Expected a work-in-progress root. This is a bug in React. Please file an issue.",
   "350": "Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.",
-  "351": "Unsupported type.",
+  "351": "Unsupported server component type: %s",
   "352": "React Blocks (and Lazy Components) are expected to be replaced by a compiler on the server. Try configuring your compiler set up and avoid using React.lazy inside of Blocks.",
   "353": "A server block should never encode any other slots. This is a bug in React.",
   "354": "getInspectorDataForViewAtPoint() is not available in production.",
@@ -366,5 +366,6 @@
   "375": "Functions cannot be passed directly to client components because they're not serializable. Remove %s (%s) from this object, or avoid the entire object: %s",
   "376": "Symbol values (%s) cannot be passed to client components. Remove %s from this object, or avoid the entire object: %s",
   "377": "BigInt (%s) is not yet supported in client component props. Remove %s from this object or use a plain number instead: %s",
-  "378": "Type %s is not supported in client component props. Remove %s from this object, or avoid the entire object: %s"
+  "378": "Type %s is not supported in client component props. Remove %s from this object, or avoid the entire object: %s",
+  "379": "Refs cannot be used in server components, nor passed to client components."
 }
