@samitouri / QOS-React-2 / commits / 40c52de960

[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

Sebastian Markbåge committed Oct 8, 2020 at 14:11 UTC 40c52de96043f56430d464a62635014f0e8dd900
3 files changed +368 -6
packages/react-client/src/__tests__/ReactFlight-test.js
+121
@@ -18,6 +18,7 @@ let ReactNoop;
18 let ReactNoopFlightServer;
19 let ReactNoopFlightServerRuntime;
20 let ReactNoopFlightClient;
21 +let ErrorBoundary;
22
23 describe('ReactFlight', () => {
24 beforeEach(() => {
@@ -29,6 +30,27 @@ describe('ReactFlight', () => {
30 ReactNoopFlightServerRuntime = require('react-noop-renderer/flight-server-runtime');
31 ReactNoopFlightClient = require('react-noop-renderer/flight-client');
32 act = ReactNoop.act;
33 +
34 + ErrorBoundary = class extends React.Component {
35 + state = {hasError: false, error: null};
36 + static getDerivedStateFromError(error) {
37 + return {
38 + hasError: true,
39 + error,
40 + };
41 + }
42 + componentDidMount() {
43 + expect(this.state.hasError).toBe(true);
44 + expect(this.state.error).toBeTruthy();
45 + expect(this.state.error.message).toContain(this.props.expectedMessage);
46 + }
47 + render() {
48 + if (this.state.hasError) {
49 + return this.state.error.message;
50 + }
51 + return this.props.children;
52 + }
53 + };
54 });
55
56 function block(render, load) {
@@ -127,4 +149,103 @@ describe('ReactFlight', () => {
149 expect(ReactNoop).toMatchRenderedOutput(<span>Hello, Seb Smith</span>);
150 });
151 }
152 +
153 + it('should error if a non-serializable value is passed to a host component', () => {
154 + function EventHandlerProp() {
155 + return (
156 + <div className="foo" onClick={function() {}}>
157 + Test
158 + </div>
159 + );
160 + }
161 + function FunctionProp() {
162 + return <div>{() => {}}</div>;
163 + }
164 + function SymbolProp() {
165 + return <div foo={Symbol('foo')} />;
166 + }
167 +
168 + const event = ReactNoopFlightServer.render(<EventHandlerProp />);
169 + const fn = ReactNoopFlightServer.render(<FunctionProp />);
170 + const symbol = ReactNoopFlightServer.render(<SymbolProp />);
171 +
172 + function Client({transport}) {
173 + return ReactNoopFlightClient.read(transport);
174 + }
175 +
176 + act(() => {
177 + ReactNoop.render(
178 + <>
179 + <ErrorBoundary expectedMessage="Event handlers cannot be passed to client component props.">
180 + <Client transport={event} />
181 + </ErrorBoundary>
182 + <ErrorBoundary expectedMessage="Functions cannot be passed directly to client components because they're not serializable.">
183 + <Client transport={fn} />
184 + </ErrorBoundary>
185 + <ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
186 + <Client transport={symbol} />
187 + </ErrorBoundary>
188 + </>,
189 + );
190 + });
191 + });
192 +
193 + it('should warn in DEV if a toJSON instance is passed to a host component', () => {
194 + expect(() => {
195 + const transport = ReactNoopFlightServer.render(
196 + <input value={new Date()} />,
197 + );
198 + act(() => {
199 + ReactNoop.render(ReactNoopFlightClient.read(transport));
200 + });
201 + }).toErrorDev(
202 + 'Only plain objects can be passed to client components from server components. ',
203 + {withoutStack: true},
204 + );
205 + });
206 +
207 + it('should warn in DEV if a special object is passed to a host component', () => {
208 + expect(() => {
209 + const transport = ReactNoopFlightServer.render(<input value={Math} />);
210 + act(() => {
211 + ReactNoop.render(ReactNoopFlightClient.read(transport));
212 + });
213 + }).toErrorDev(
214 + 'Only plain objects can be passed to client components from server components. ' +
215 + 'Built-ins like Math are not supported.',
216 + {withoutStack: true},
217 + );
218 + });
219 +
220 + it('should warn in DEV if an object with symbols is passed to a host component', () => {
221 + expect(() => {
222 + const transport = ReactNoopFlightServer.render(
223 + <input value={{[Symbol.iterator]: {}}} />,
224 + );
225 + act(() => {
226 + ReactNoop.render(ReactNoopFlightClient.read(transport));
227 + });
228 + }).toErrorDev(
229 + 'Only plain objects can be passed to client components from server components. ' +
230 + 'Objects with symbol properties like Symbol.iterator are not supported.',
231 + {withoutStack: true},
232 + );
233 + });
234 +
235 + it('should warn in DEV if a class instance is passed to a host component', () => {
236 + class Foo {
237 + method() {}
238 + }
239 + expect(() => {
240 + const transport = ReactNoopFlightServer.render(
241 + <input value={new Foo()} />,
242 + );
243 + act(() => {
244 + ReactNoop.render(ReactNoopFlightClient.read(transport));
245 + });
246 + }).toErrorDev(
247 + 'Only plain objects can be passed to client components from server components. ',
248 + {withoutStack: true},
249 + );
250 + });
251 });
packages/react-server/src/ReactFlightServer.js
+241 -5
@@ -50,6 +50,8 @@ import * as React from 'react';
50 import ReactSharedInternals from 'shared/ReactSharedInternals';
51 import invariant from 'shared/invariant';
52
53 +const isArray = Array.isArray;
54 +
55 type ReactJSONValue =
56 | string
57 | boolean
@@ -186,12 +188,146 @@ function escapeStringValue(value: string): string {
188 }
189 }
190
191 +function isObjectPrototype(object): boolean {
192 + if (!object) {
193 + return false;
194 + }
195 + // $FlowFixMe
196 + const ObjectPrototype = Object.prototype;
197 + if (object === ObjectPrototype) {
198 + return true;
199 + }
200 + // It might be an object from a different Realm which is
201 + // still just a plain simple object.
202 + if (Object.getPrototypeOf(object)) {
203 + return false;
204 + }
205 + const names = Object.getOwnPropertyNames(object);
206 + for (let i = 0; i < names.length; i++) {
207 + if (!(names[i] in ObjectPrototype)) {
208 + return false;
209 + }
210 + }
211 + return true;
212 +}
213 +
214 +function isSimpleObject(object): boolean {
215 + if (!isObjectPrototype(Object.getPrototypeOf(object))) {
216 + return false;
217 + }
218 + const names = Object.getOwnPropertyNames(object);
219 + for (let i = 0; i < names.length; i++) {
220 + const descriptor = Object.getOwnPropertyDescriptor(object, names[i]);
221 + if (!descriptor || !descriptor.enumerable) {
222 + return false;
223 + }
224 + }
225 + return true;
226 +}
227 +
228 +function objectName(object): string {
229 + const name = Object.prototype.toString.call(object);
230 + return name.replace(/^\[object (.*)\]$/, function(m, p0) {
231 + return p0;
232 + });
233 +}
234 +
235 +function describeKeyForErrorMessage(key: string): string {
236 + const encodedKey = JSON.stringify(key);
237 + return '"' + key + '"' === encodedKey ? key : encodedKey;
238 +}
239 +
240 +function describeValueForErrorMessage(value: ReactModel): string {
241 + switch (typeof value) {
242 + case 'string': {
243 + return JSON.stringify(
244 + value.length <= 10 ? value : value.substr(0, 10) + '...',
245 + );
246 + }
247 + case 'object': {
248 + if (isArray(value)) {
249 + return '[...]';
250 + }
251 + const name = objectName(value);
252 + if (name === '[object Object]') {
253 + return '{...}';
254 + }
255 + return name;
256 + }
257 + case 'function':
258 + return 'function';
259 + default:
260 + // eslint-disable-next-line
261 + return String(value);
262 + }
263 +}
264 +
265 +function describeObjectForErrorMessage(
266 + objectOrArray:
267 + | {+[key: string | number]: ReactModel}
268 + | $ReadOnlyArray<ReactModel>,
269 +): string {
270 + if (isArray(objectOrArray)) {
271 + let str = '[';
272 + // $FlowFixMe: Should be refined by now.
273 + const array: $ReadOnlyArray<ReactModel> = objectOrArray;
274 + for (let i = 0; i < array.length; i++) {
275 + if (i > 0) {
276 + str += ', ';
277 + }
278 + if (i > 6) {
279 + str += '...';
280 + break;
281 + }
282 + str += describeValueForErrorMessage(array[i]);
283 + }
284 + str += ']';
285 + return str;
286 + } else {
287 + let str = '{';
288 + // $FlowFixMe: Should be refined by now.
289 + const object: {+[key: string | number]: ReactModel} = objectOrArray;
290 + const names = Object.keys(object);
291 + for (let i = 0; i < names.length; i++) {
292 + if (i > 0) {
293 + str += ', ';
294 + }
295 + if (i > 6) {
296 + str += '...';
297 + break;
298 + }
299 + const name = names[i];
300 + str +=
301 + describeKeyForErrorMessage(name) +
302 + ': ' +
303 + describeValueForErrorMessage(object[name]);
304 + }
305 + str += '}';
306 + return str;
307 + }
308 +}
309 +
310 export function resolveModelToJSON(
311 request: Request,
312 parent: {+[key: string | number]: ReactModel} | $ReadOnlyArray<ReactModel>,
313 key: string,
314 value: ReactModel,
315 ): ReactJSONValue {
316 + if (__DEV__) {
317 + // $FlowFixMe
318 + const originalValue = parent[key];
319 + if (typeof originalValue === 'object' && originalValue !== value) {
320 + console.error(
321 + 'Only plain objects can be passed to client components from server components. ' +
322 + 'Objects with toJSON methods are not supported. Convert it manually ' +
323 + 'to a simple value before passing it to props. ' +
324 + 'Remove %s from these props: %s',
325 + describeKeyForErrorMessage(key),
326 + describeObjectForErrorMessage(parent),
327 + );
328 + }
329 + }
330 +
331 // Special Symbols
332 switch (value) {
333 case REACT_ELEMENT_TYPE:
@@ -263,10 +399,6 @@ export function resolveModelToJSON(
399 }
400 }
401
266 - if (typeof value === 'string') {
267 - return escapeStringValue(value);
268 - }
269 -
402 // Resolve server components.
403 while (
404 typeof value === 'object' &&
@@ -293,7 +425,111 @@ export function resolveModelToJSON(
425 }
426 }
427
296 - return value;
428 + if (typeof value === 'object') {
429 + if (__DEV__) {
430 + if (value !== null && !isArray(value)) {
431 + // Verify that this is a simple plain object.
432 + if (objectName(value) !== 'Object') {
433 + console.error(
434 + 'Only plain objects can be passed to client components from server components. ' +
435 + 'Built-ins like %s are not supported. ' +
436 + 'Remove %s from these props: %s',
437 + objectName(value),
438 + describeKeyForErrorMessage(key),
439 + describeObjectForErrorMessage(parent),
440 + );
441 + } else if (!isSimpleObject(value)) {
442 + console.error(
443 + 'Only plain objects can be passed to client components from server components. ' +
444 + 'Classes or other objects with methods are not supported. ' +
445 + 'Remove %s from these props: %s',
446 + describeKeyForErrorMessage(key),
447 + describeObjectForErrorMessage(parent),
448 + );
449 + } else if (Object.getOwnPropertySymbols) {
450 + const symbols = Object.getOwnPropertySymbols(value);
451 + if (symbols.length > 0) {
452 + console.error(
453 + 'Only plain objects can be passed to client components from server components. ' +
454 + 'Objects with symbol properties like %s are not supported. ' +
455 + 'Remove %s from these props: %s',
456 + symbols[0].description,
457 + describeKeyForErrorMessage(key),
458 + describeObjectForErrorMessage(parent),
459 + );
460 + }
461 + }
462 + }
463 + }
464 + return value;
465 + }
466 +
467 + if (typeof value === 'string') {
468 + return escapeStringValue(value);
469 + }
470 +
471 + if (
472 + typeof value === 'boolean' ||
473 + typeof value === 'number' ||
474 + typeof value === 'undefined'
475 + ) {
476 + return value;
477 + }
478 +
479 + if (typeof value === 'function') {
480 + if (/^on[A-Z]/.test(key)) {
481 + invariant(
482 + false,
483 + 'Event handlers cannot be passed to client component props. ' +
484 + 'Remove %s from these props if possible: %s\n' +
485 + 'If you need interactivity, consider converting part of this to a client component.',
486 + describeKeyForErrorMessage(key),
487 + describeObjectForErrorMessage(parent),
488 + );
489 + } else {
490 + invariant(
491 + false,
492 + 'Functions cannot be passed directly to client components ' +
493 + "because they're not serializable. " +
494 + 'Remove %s (%s) from this object, or avoid the entire object: %s',
495 + describeKeyForErrorMessage(key),
496 + value.displayName || value.name || 'function',
497 + describeObjectForErrorMessage(parent),
498 + );
499 + }
500 + }
501 +
502 + if (typeof value === 'symbol') {
503 + invariant(
504 + false,
505 + 'Symbol values (%s) cannot be passed to client components. ' +
506 + 'Remove %s from this object, or avoid the entire object: %s',
507 + value.description,
508 + describeKeyForErrorMessage(key),
509 + describeObjectForErrorMessage(parent),
510 + );
511 + }
512 +
513 + // $FlowFixMe: bigint isn't added to Flow yet.
514 + if (typeof value === 'bigint') {
515 + invariant(
516 + false,
517 + 'BigInt (%s) is not yet supported in client component props. ' +
518 + 'Remove %s from this object or use a plain number instead: %s',
519 + value,
520 + describeKeyForErrorMessage(key),
521 + describeObjectForErrorMessage(parent),
522 + );
523 + }
524 +
525 + invariant(
526 + false,
527 + 'Type %s is not supported in client component props. ' +
528 + 'Remove %s from this object, or avoid the entire object: %s',
529 + typeof value,
530 + describeKeyForErrorMessage(key),
531 + describeObjectForErrorMessage(parent),
532 + );
533 }
534
535 function emitErrorChunk(request: Request, id: number, error: mixed): void {
scripts/error-codes/codes.json
+6 -1
@@ -361,5 +361,10 @@
361 "370": "ReactDOM.createEventHandle: setter called with an invalid callback. The callback must be a function.",
362 "371": "Text string must be rendered within a <Text> component.\n\nText: %s",
363 "372": "Cannot call unstable_createEventHandle with \"%s\", as it is not an event known to React.",
364 - "373": "This Hook is not supported in Server Components."
364 + "373": "This Hook is not supported in Server Components.",
365 + "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.",
366 + "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",
367 + "376": "Symbol values (%s) cannot be passed to client components. Remove %s from this object, or avoid the entire object: %s",
368 + "377": "BigInt (%s) is not yet supported in client component props. Remove %s from this object or use a plain number instead: %s",
369 + "378": "Type %s is not supported in client component props. Remove %s from this object, or avoid the entire object: %s"
370 }