Skip to content

[Flight] Add Runtime Errors for Non-serializable Values #19980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 8, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
Original file line number Diff line number Diff line change
@@ -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},
);
});
});
246 changes: 241 additions & 5 deletions packages/react-server/src/ReactFlightServer.js
Original file line number Diff line number Diff line change
@@ -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 {
7 changes: 6 additions & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
@@ -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"
}