Skip to content

Commit

Permalink
[Flight] Improve Error Messages when Invalid Object is Passed to Clie…
Browse files Browse the repository at this point in the history 
…nt/Host Components (#25492)

* Print built-in specific error message for toJSON

This is a better message for Date.

Also, format the message to highlight the affected prop.

* Describe error messages using JSX elements in DEV

We don't have access to the grand parent objects on the stack so we stash
them on weakmaps so we can access them while printing error messages.

Might be a bit slow.

* Capitalize Server/Client Component

* Special case errror messages for children of host components

These are likely meant to be text content if they're not a supported object.

* Update error messages
  • Loading branch information
@sebmarkbage @rickhanlonii
sebmarkbage authored and rickhanlonii committed Dec 3, 2022
1 parent f59465f commit de8901f
Show file tree
Hide file tree
Showing 6 changed files with 445 additions and 150 deletions.
231 changes: 203 additions & 28 deletions packages/react-client/src/__tests__/ReactFlight-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('ReactFlight', () => {
};
}

it('can render a server component', async () => {
it('can render a Server Component', async () => {
function Bar({text}) {
return text.toUpperCase();
}
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('ReactFlight', () => {
});
});

it('can render a client component using a module reference and render there', async () => {
it('can render a Client Component using a module reference and render there', async () => {
function UserClient(props) {
return (
<span>
Expand Down Expand Up @@ -363,6 +363,11 @@ describe('ReactFlight', () => {

// @gate enableUseHook
it('should error if a non-serializable value is passed to a host component', async () => {
function ClientImpl({children}) {
return children;
}
const Client = moduleReference(ClientImpl);

function EventHandlerProp() {
return (
<div className="foo" onClick={function() {}}>
Expand All @@ -382,6 +387,24 @@ describe('ReactFlight', () => {
return <div ref={ref} />;
}

function EventHandlerPropClient() {
return (
<Client className="foo" onClick={function() {}}>
Test
</Client>
);
}
function FunctionPropClient() {
return <Client>{() => {}}</Client>;
}
function SymbolPropClient() {
return <Client foo={Symbol('foo')} />;
}

function RefPropClient() {
return <Client ref={ref} />;
}

const options = {
onError(x) {
return __DEV__ ? 'a dev digest' : `digest("${x.message}")`;
Expand All @@ -391,26 +414,51 @@ describe('ReactFlight', () => {
const fn = ReactNoopFlightServer.render(<FunctionProp />, options);
const symbol = ReactNoopFlightServer.render(<SymbolProp />, options);
const refs = ReactNoopFlightServer.render(<RefProp />, options);
const eventClient = ReactNoopFlightServer.render(
<EventHandlerPropClient />,
options,
);
const fnClient = ReactNoopFlightServer.render(
<FunctionPropClient />,
options,
);
const symbolClient = ReactNoopFlightServer.render(
<SymbolPropClient />,
options,
);
const refsClient = ReactNoopFlightServer.render(<RefPropClient />, options);

function Client({promise}) {
function Render({promise}) {
return use(promise);
}

await act(async () => {
startTransition(() => {
ReactNoop.render(
<>
<ErrorBoundary expectedMessage="Event handlers cannot be passed to client component props.">
<Client promise={ReactNoopFlightClient.read(event)} />
<ErrorBoundary expectedMessage="Event handlers cannot be passed to Client Component props.">
<Render promise={ReactNoopFlightClient.read(event)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Functions cannot be passed directly to Client Components because they're not serializable.">
<Render promise={ReactNoopFlightClient.read(fn)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Only global symbols received from Symbol.for(...) can be passed to Client Components.">
<Render promise={ReactNoopFlightClient.read(symbol)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Refs cannot be used in Server Components, nor passed to Client Components.">
<Render promise={ReactNoopFlightClient.read(refs)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Event handlers cannot be passed to Client Component props.">
<Render promise={ReactNoopFlightClient.read(eventClient)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Functions cannot be passed directly to client components because they're not serializable.">
<Client promise={ReactNoopFlightClient.read(fn)} />
<ErrorBoundary expectedMessage="Functions cannot be passed directly to Client Components because they're not serializable.">
<Render promise={ReactNoopFlightClient.read(fnClient)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Only global symbols received from Symbol.for(...) can be passed to client components.">
<Client promise={ReactNoopFlightClient.read(symbol)} />
<ErrorBoundary expectedMessage="Only global symbols received from Symbol.for(...) can be passed to Client Components.">
<Render promise={ReactNoopFlightClient.read(symbolClient)} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Refs cannot be used in server components, nor passed to client components.">
<Client promise={ReactNoopFlightClient.read(refs)} />
<ErrorBoundary expectedMessage="Refs cannot be used in Server Components, nor passed to Client Components.">
<Render promise={ReactNoopFlightClient.read(refsClient)} />
</ErrorBoundary>
</>,
);
Expand All @@ -419,19 +467,19 @@ describe('ReactFlight', () => {
});

// @gate enableUseHook
it('should trigger the inner most error boundary inside a client component', async () => {
it('should trigger the inner most error boundary inside a Client Component', async () => {
function ServerComponent() {
throw new Error('This was thrown in the server component.');
throw new Error('This was thrown in the Server Component.');
}

function ClientComponent({children}) {
// This should catch the error thrown by the server component, even though it has already happened.
// This should catch the error thrown by the Server Component, even though it has already happened.
// We currently need to wrap it in a div because as it's set up right now, a lazy reference will
// throw during reconciliation which will trigger the parent of the error boundary.
// This is similar to how these will suspend the parent if it's a direct child of a Suspense boundary.
// That's a bug.
return (
<ErrorBoundary expectedMessage="This was thrown in the server component.">
<ErrorBoundary expectedMessage="This was thrown in the Server Component.">
<div>{children}</div>
</ErrorBoundary>
);
Expand Down Expand Up @@ -475,25 +523,37 @@ describe('ReactFlight', () => {
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
'Only plain objects can be passed to Client Components from Server Components. ' +
'Date objects are not supported.',
{withoutStack: true},
);
});

it('should warn in DEV if a special object is passed to a host component', () => {
it('should warn in DEV if a toJSON instance is passed to a host component child', () => {
expect(() => {
const transport = ReactNoopFlightServer.render(<input value={Math} />);
const transport = ReactNoopFlightServer.render(
<div>Current date: {new Date()}</div>,
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ' +
'Built-ins like Math are not supported.',
'Date objects cannot be rendered as text children. Try formatting it using toString().\n' +
' <div>Current date: {Date}</div>\n' +
' ^^^^^^',
{withoutStack: true},
);
});

it('should NOT warn in DEV for key getters', () => {
const transport = ReactNoopFlightServer.render(<div key="a" />);
ReactNoopFlightClient.read(transport);
it('should warn in DEV if a special object is passed to a host component', () => {
expect(() => {
const transport = ReactNoopFlightServer.render(<input value={Math} />);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Math objects are not supported.\n' +
' <input value={Math}>\n' +
' ^^^^^^',
{withoutStack: true},
);
});

it('should warn in DEV if an object with symbols is passed to a host component', () => {
Expand All @@ -503,12 +563,127 @@ describe('ReactFlight', () => {
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ' +
'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 toJSON instance is passed to a Client Component', () => {
function ClientImpl({value}) {
return <div>{value}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(
<Client value={new Date()} />,
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Date objects are not supported.',
{withoutStack: true},
);
});

it('should warn in DEV if a toJSON instance is passed to a Client Component child', () => {
function ClientImpl({children}) {
return <div>{children}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(
<Client>Current date: {new Date()}</Client>,
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Date objects are not supported.\n' +
' <>Current date: {Date}</>\n' +
' ^^^^^^',
{withoutStack: true},
);
});

it('should warn in DEV if a special object is passed to a Client Component', () => {
function ClientImpl({value}) {
return <div>{value}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(<Client value={Math} />);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Math objects are not supported.\n' +
' <... value={Math}>\n' +
' ^^^^^^',
{withoutStack: true},
);
});

it('should warn in DEV if an object with symbols is passed to a Client Component', () => {
function ClientImpl({value}) {
return <div>{value}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(
<Client value={{[Symbol.iterator]: {}}} />,
);
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 special object is passed to a nested object in Client Component', () => {
function ClientImpl({value}) {
return <div>{value}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(
<Client value={{hello: Math, title: <h1>hi</h1>}} />,
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Math objects are not supported.\n' +
' {hello: Math, title: <h1/>}\n' +
' ^^^^',
{withoutStack: true},
);
});

it('should warn in DEV if a special object is passed to a nested array in Client Component', () => {
function ClientImpl({value}) {
return <div>{value}</div>;
}
const Client = moduleReference(ClientImpl);
expect(() => {
const transport = ReactNoopFlightServer.render(
<Client
value={['looooong string takes up noise', Math, <h1>hi</h1>]}
/>,
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to Client Components from Server Components. ' +
'Math objects are not supported.\n' +
' [..., Math, <h1/>]\n' +
' ^^^^',
{withoutStack: true},
);
});

it('should NOT warn in DEV for key getters', () => {
const transport = ReactNoopFlightServer.render(<div key="a" />);
ReactNoopFlightClient.read(transport);
});

it('should warn in DEV if a class instance is passed to a host component', () => {
class Foo {
method() {}
Expand All @@ -519,7 +694,7 @@ describe('ReactFlight', () => {
);
ReactNoopFlightClient.read(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
'Only plain objects can be passed to Client Components from Server Components. ',
{withoutStack: true},
);
});
Expand Down Expand Up @@ -577,9 +752,9 @@ describe('ReactFlight', () => {
});

it('[TODO] it does not warn if you render a server element passed to a client module reference twice on the client when using useId', async () => {
// @TODO Today if you render a server component with useId and pass it to a client component and that client component renders the element in two or more
// @TODO Today if you render a Server Component with useId and pass it to a Client Component and that Client Component renders the element in two or more
// places the id used on the server will be duplicated in the client. This is a deviation from the guarantees useId makes for Fizz/Client and is a consequence
// of the fact that the server component is actually rendered on the server and is reduced to a set of host elements before being passed to the Client component
// of the fact that the Server Component is actually rendered on the server and is reduced to a set of host elements before being passed to the Client component
// so the output passed to the Client has no knowledge of the useId use. In the future we would like to add a DEV warning when this happens. For now
// we just accept that it is a nuance of useId in Flight
function App() {
Expand Down Expand Up @@ -937,7 +1112,7 @@ describe('ReactFlight', () => {

expect(ClientContext).toBe(undefined);

// Reset all modules, except flight-modules which keeps the registry of client components
// Reset all modules, except flight-modules which keeps the registry of Client Components
const flightModules = require('react-noop-renderer/flight-modules');
jest.resetModules();
jest.mock('react-noop-renderer/flight-modules', () => flightModules);
Expand Down
6 changes: 3 additions & 3 deletions packages/react-server-dom-relay/src/__tests__/ReactFlightDOMRelay-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('ReactFlightDOMRelay', () => {
return model;
}

it('can render a server component', () => {
it('can render a Server Component', () => {
function Bar({text}) {
return text.toUpperCase();
}
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('ReactFlightDOMRelay', () => {
});
});

it('can render a client component using a module reference and render there', () => {
it('can render a Client Component using a module reference and render there', () => {
function UserClient(props) {
return (
<span>
Expand Down Expand Up @@ -233,7 +233,7 @@ describe('ReactFlightDOMRelay', () => {
ReactDOMFlightRelayServer.render(<input value={new Foo()} />, transport);
readThrough(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
'Only plain objects can be passed to Client Components from Server Components. ',
{withoutStack: true},
);
});
Expand Down
6 changes: 3 additions & 3 deletions packages/react-server-native-relay/src/__tests__/ReactFlightNativeRelay-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('ReactFlightNativeRelay', () => {
return model;
}

it('can render a server component', () => {
it('can render a Server Component', () => {
function Bar({text}) {
return <Text>{text.toUpperCase()}</Text>;
}
Expand All @@ -86,7 +86,7 @@ describe('ReactFlightNativeRelay', () => {
expect(model).toMatchSnapshot();
});

it('can render a client component using a module reference and render there', () => {
it('can render a Client Component using a module reference and render there', () => {
function UserClient(props) {
return (
<Text>
Expand Down Expand Up @@ -132,7 +132,7 @@ describe('ReactFlightNativeRelay', () => {
);
readThrough(transport);
}).toErrorDev(
'Only plain objects can be passed to client components from server components. ',
'Only plain objects can be passed to Client Components from Server Components. ',
{withoutStack: true},
);
});
Expand Down
Loading

0 comments on commit de8901f

Please sign in to comment.