From 40c52de96043f56430d464a62635014f0e8dd900 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= <sema@fb.com>
Date: Thu, 8 Oct 2020 14:11:15 -0400
Subject: [PATCH] [Flight] Add Runtime Errors for Non-serializable Values
 (#19980)

* Error on encoding non-serializable props

* Add DEV time warnings to enforce that values are plain objects
---
 .../src/__tests__/ReactFlight-test.js         | 121 +++++++++
 .../react-server/src/ReactFlightServer.js     | 246 +++++++++++++++++-
 scripts/error-codes/codes.json                |   7 +-
 3 files changed, 368 insertions(+), 6 deletions(-)

diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
index 8e877ea101644..e08e61748cd4d 100644
--- a/packages/react-client/src/__tests__/ReactFlight-test.js
+++ b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -18,6 +18,7 @@ let ReactNoop;
 let ReactNoopFlightServer;
 let ReactNoopFlightServerRuntime;
 let ReactNoopFlightClient;
+let ErrorBoundary;
 
 describe('ReactFlight', () => {
   beforeEach(() => {
@@ -29,6 +30,27 @@ describe('ReactFlight', () => {
     ReactNoopFlightServerRuntime = require('react-noop-renderer/flight-server-runtime');
     ReactNoopFlightClient = require('react-noop-renderer/flight-client');
     act = ReactNoop.act;
+
+    ErrorBoundary = class extends React.Component {
+      state = {hasError: false, error: null};
+      static getDerivedStateFromError(error) {
+        return {
+          hasError: true,
+          error,
+        };
+      }
+      componentDidMount() {
+        expect(this.state.hasError).toBe(true);
+        expect(this.state.error).toBeTruthy();
+        expect(this.state.error.message).toContain(this.props.expectedMessage);
+      }
+      render() {
+        if (this.state.hasError) {
+          return this.state.error.message;
+        }
+        return this.props.children;
+      }
+    };
   });
 
   function block(render, load) {
@@ -127,4 +149,103 @@ describe('ReactFlight', () => {
       expect(ReactNoop).toMatchRenderedOutput(<span>Hello, Seb Smith</span>);
     });
   }
+
+  it('should error if a non-serializable value is passed to a host component', () => {
+    function EventHandlerProp() {
+      return (
+        <div className="foo" onClick={function() {}}>
+          Test
+        </div>
+      );
+    }
+    function FunctionProp() {
+      return <div>{() => {}}</div>;
+    }
+    function SymbolProp() {
+      return <div foo={Symbol('foo')} />;
+    }
+
+    const event = ReactNoopFlightServer.render(<EventHandlerProp />);
+    const fn = ReactNoopFlightServer.render(<FunctionProp />);
+    const symbol = ReactNoopFlightServer.render(<SymbolProp />);
+
+    function Client({transport}) {
+      return ReactNoopFlightClient.read(transport);
+    }
+
+    act(() => {
+      ReactNoop.render(
+        <>
+          <ErrorBoundary expectedMessage="Event handlers cannot be passed to client component props.">
+            <Client transport={event} />
+          </ErrorBoundary>
+          <ErrorBoundary expectedMessage="Functions cannot be passed directly to client components because they're not serializable.">
+            <Client transport={fn} />
+          </ErrorBoundary>
+          <ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
+            <Client transport={symbol} />
+          </ErrorBoundary>
+        </>,
+      );
+    });
+  });
+
+  it('should warn in DEV if a toJSON instance is passed to a host component', () => {
+    expect(() => {
+      const transport = ReactNoopFlightServer.render(
+        <input value={new Date()} />,
+      );
+      act(() => {
+        ReactNoop.render(ReactNoopFlightClient.read(transport));
+      });
+    }).toErrorDev(
+      'Only plain objects can be passed to client components from server components. ',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn in DEV if a special object is passed to a host component', () => {
+    expect(() => {
+      const transport = ReactNoopFlightServer.render(<input value={Math} />);
+      act(() => {
+        ReactNoop.render(ReactNoopFlightClient.read(transport));
+      });
+    }).toErrorDev(
+      'Only plain objects can be passed to client components from server components. ' +
+        'Built-ins like Math are not supported.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn in DEV if an object with symbols is passed to a host component', () => {
+    expect(() => {
+      const transport = ReactNoopFlightServer.render(
+        <input value={{[Symbol.iterator]: {}}} />,
+      );
+      act(() => {
+        ReactNoop.render(ReactNoopFlightClient.read(transport));
+      });
+    }).toErrorDev(
+      'Only plain objects can be passed to client components from server components. ' +
+        'Objects with symbol properties like Symbol.iterator are not supported.',
+      {withoutStack: true},
+    );
+  });
+
+  it('should warn in DEV if a class instance is passed to a host component', () => {
+    class Foo {
+      method() {}
+    }
+    expect(() => {
+      const transport = ReactNoopFlightServer.render(
+        <input value={new Foo()} />,
+      );
+      act(() => {
+        ReactNoop.render(ReactNoopFlightClient.read(transport));
+      });
+    }).toErrorDev(
+      'Only plain objects can be passed to client components from server components. ',
+      {withoutStack: true},
+    );
+  });
 });
diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
index 17fd077c57f88..d934ce5f7fbf7 100644
--- a/packages/react-server/src/ReactFlightServer.js
+++ b/packages/react-server/src/ReactFlightServer.js
@@ -50,6 +50,8 @@ import * as React from 'react';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
 
+const isArray = Array.isArray;
+
 type ReactJSONValue =
   | string
   | boolean
@@ -186,12 +188,146 @@ function escapeStringValue(value: string): string {
   }
 }
 
+function isObjectPrototype(object): boolean {
+  if (!object) {
+    return false;
+  }
+  // $FlowFixMe
+  const ObjectPrototype = Object.prototype;
+  if (object === ObjectPrototype) {
+    return true;
+  }
+  // It might be an object from a different Realm which is
+  // still just a plain simple object.
+  if (Object.getPrototypeOf(object)) {
+    return false;
+  }
+  const names = Object.getOwnPropertyNames(object);
+  for (let i = 0; i < names.length; i++) {
+    if (!(names[i] in ObjectPrototype)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+function isSimpleObject(object): boolean {
+  if (!isObjectPrototype(Object.getPrototypeOf(object))) {
+    return false;
+  }
+  const names = Object.getOwnPropertyNames(object);
+  for (let i = 0; i < names.length; i++) {
+    const descriptor = Object.getOwnPropertyDescriptor(object, names[i]);
+    if (!descriptor || !descriptor.enumerable) {
+      return false;
+    }
+  }
+  return true;
+}
+
+function objectName(object): string {
+  const name = Object.prototype.toString.call(object);
+  return name.replace(/^\[object (.*)\]$/, function(m, p0) {
+    return p0;
+  });
+}
+
+function describeKeyForErrorMessage(key: string): string {
+  const encodedKey = JSON.stringify(key);
+  return '"' + key + '"' === encodedKey ? key : encodedKey;
+}
+
+function describeValueForErrorMessage(value: ReactModel): string {
+  switch (typeof value) {
+    case 'string': {
+      return JSON.stringify(
+        value.length <= 10 ? value : value.substr(0, 10) + '...',
+      );
+    }
+    case 'object': {
+      if (isArray(value)) {
+        return '[...]';
+      }
+      const name = objectName(value);
+      if (name === '[object Object]') {
+        return '{...}';
+      }
+      return name;
+    }
+    case 'function':
+      return 'function';
+    default:
+      // eslint-disable-next-line
+      return String(value);
+  }
+}
+
+function describeObjectForErrorMessage(
+  objectOrArray:
+    | {+[key: string | number]: ReactModel}
+    | $ReadOnlyArray<ReactModel>,
+): string {
+  if (isArray(objectOrArray)) {
+    let str = '[';
+    // $FlowFixMe: Should be refined by now.
+    const array: $ReadOnlyArray<ReactModel> = objectOrArray;
+    for (let i = 0; i < array.length; i++) {
+      if (i > 0) {
+        str += ', ';
+      }
+      if (i > 6) {
+        str += '...';
+        break;
+      }
+      str += describeValueForErrorMessage(array[i]);
+    }
+    str += ']';
+    return str;
+  } else {
+    let str = '{';
+    // $FlowFixMe: Should be refined by now.
+    const object: {+[key: string | number]: ReactModel} = objectOrArray;
+    const names = Object.keys(object);
+    for (let i = 0; i < names.length; i++) {
+      if (i > 0) {
+        str += ', ';
+      }
+      if (i > 6) {
+        str += '...';
+        break;
+      }
+      const name = names[i];
+      str +=
+        describeKeyForErrorMessage(name) +
+        ': ' +
+        describeValueForErrorMessage(object[name]);
+    }
+    str += '}';
+    return str;
+  }
+}
+
 export function resolveModelToJSON(
   request: Request,
   parent: {+[key: string | number]: ReactModel} | $ReadOnlyArray<ReactModel>,
   key: string,
   value: ReactModel,
 ): ReactJSONValue {
+  if (__DEV__) {
+    // $FlowFixMe
+    const originalValue = parent[key];
+    if (typeof originalValue === 'object' && originalValue !== value) {
+      console.error(
+        'Only plain objects can be passed to client components from server components. ' +
+          'Objects with toJSON methods are not supported. Convert it manually ' +
+          'to a simple value before passing it to props. ' +
+          'Remove %s from these props: %s',
+        describeKeyForErrorMessage(key),
+        describeObjectForErrorMessage(parent),
+      );
+    }
+  }
+
   // Special Symbols
   switch (value) {
     case REACT_ELEMENT_TYPE:
@@ -263,10 +399,6 @@ export function resolveModelToJSON(
     }
   }
 
-  if (typeof value === 'string') {
-    return escapeStringValue(value);
-  }
-
   // Resolve server components.
   while (
     typeof value === 'object' &&
@@ -293,7 +425,111 @@ export function resolveModelToJSON(
     }
   }
 
-  return value;
+  if (typeof value === 'object') {
+    if (__DEV__) {
+      if (value !== null && !isArray(value)) {
+        // Verify that this is a simple plain object.
+        if (objectName(value) !== 'Object') {
+          console.error(
+            'Only plain objects can be passed to client components from server components. ' +
+              'Built-ins like %s are not supported. ' +
+              'Remove %s from these props: %s',
+            objectName(value),
+            describeKeyForErrorMessage(key),
+            describeObjectForErrorMessage(parent),
+          );
+        } else if (!isSimpleObject(value)) {
+          console.error(
+            'Only plain objects can be passed to client components from server components. ' +
+              'Classes or other objects with methods are not supported. ' +
+              'Remove %s from these props: %s',
+            describeKeyForErrorMessage(key),
+            describeObjectForErrorMessage(parent),
+          );
+        } else if (Object.getOwnPropertySymbols) {
+          const symbols = Object.getOwnPropertySymbols(value);
+          if (symbols.length > 0) {
+            console.error(
+              'Only plain objects can be passed to client components from server components. ' +
+                'Objects with symbol properties like %s are not supported. ' +
+                'Remove %s from these props: %s',
+              symbols[0].description,
+              describeKeyForErrorMessage(key),
+              describeObjectForErrorMessage(parent),
+            );
+          }
+        }
+      }
+    }
+    return value;
+  }
+
+  if (typeof value === 'string') {
+    return escapeStringValue(value);
+  }
+
+  if (
+    typeof value === 'boolean' ||
+    typeof value === 'number' ||
+    typeof value === 'undefined'
+  ) {
+    return value;
+  }
+
+  if (typeof value === 'function') {
+    if (/^on[A-Z]/.test(key)) {
+      invariant(
+        false,
+        'Event handlers cannot be passed to client component props. ' +
+          'Remove %s from these props if possible: %s\n' +
+          'If you need interactivity, consider converting part of this to a client component.',
+        describeKeyForErrorMessage(key),
+        describeObjectForErrorMessage(parent),
+      );
+    } else {
+      invariant(
+        false,
+        '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',
+        describeKeyForErrorMessage(key),
+        value.displayName || value.name || 'function',
+        describeObjectForErrorMessage(parent),
+      );
+    }
+  }
+
+  if (typeof value === 'symbol') {
+    invariant(
+      false,
+      'Symbol values (%s) cannot be passed to client components. ' +
+        'Remove %s from this object, or avoid the entire object: %s',
+      value.description,
+      describeKeyForErrorMessage(key),
+      describeObjectForErrorMessage(parent),
+    );
+  }
+
+  // $FlowFixMe: bigint isn't added to Flow yet.
+  if (typeof value === 'bigint') {
+    invariant(
+      false,
+      'BigInt (%s) is not yet supported in client component props. ' +
+        'Remove %s from this object or use a plain number instead: %s',
+      value,
+      describeKeyForErrorMessage(key),
+      describeObjectForErrorMessage(parent),
+    );
+  }
+
+  invariant(
+    false,
+    'Type %s is not supported in client component props. ' +
+      'Remove %s from this object, or avoid the entire object: %s',
+    typeof value,
+    describeKeyForErrorMessage(key),
+    describeObjectForErrorMessage(parent),
+  );
 }
 
 function emitErrorChunk(request: Request, id: number, error: mixed): void {
diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
index 069339abfdd21..369c97c32a2cf 100644
--- a/scripts/error-codes/codes.json
+++ b/scripts/error-codes/codes.json
@@ -361,5 +361,10 @@
   "370": "ReactDOM.createEventHandle: setter called with an invalid callback. The callback must be a function.",
   "371": "Text string must be rendered within a <Text> component.\n\nText: %s",
   "372": "Cannot call unstable_createEventHandle with \"%s\", as it is not an event known to React.",
-  "373": "This Hook is not supported in Server Components."
+  "373": "This Hook is not supported in Server Components.",
+  "374": "Event handlers cannot be passed to client component props. Remove %s from these props if possible: %s\nIf you need interactivity, consider converting part of this to a client component.",
+  "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"
 }