From 7e09c2d28088ec0cc09fa45706b894f853726712 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 8 Oct 2020 18:41:31 -0400 Subject: [PATCH 1/3] Improve error message by expanding the object in question --- .../react-server/src/ReactFlightServer.js | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index d934ce5f7fb..30b0863dae9 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -249,7 +249,7 @@ function describeValueForErrorMessage(value: ReactModel): string { return '[...]'; } const name = objectName(value); - if (name === '[object Object]') { + if (name === 'Object') { return '{...}'; } return name; @@ -266,6 +266,7 @@ function describeObjectForErrorMessage( objectOrArray: | {+[key: string | number]: ReactModel} | $ReadOnlyArray, + expandedName?: string, ): string { if (isArray(objectOrArray)) { let str = '['; @@ -279,7 +280,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 +307,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 +461,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 +472,7 @@ export function resolveModelToJSON( 'Remove %s from these props: %s', symbols[0].description, describeKeyForErrorMessage(key), - describeObjectForErrorMessage(parent), + describeObjectForErrorMessage(parent, key), ); } } From 6f6f032cfbb77da1861559705acc997cfb86bfd2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 8 Oct 2020 18:51:10 -0400 Subject: [PATCH 2/3] Don't warn for key/ref getters --- .../react-client/src/__tests__/ReactFlight-test.js | 9 +++++++++ packages/react-server/src/ReactFlightServer.js | 11 ++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index e08e61748cd..34e03f4d7a1 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -217,6 +217,15 @@ describe('ReactFlight', () => { ); }); + it('should NOT warn in DEV for key/ref getters', () => { + const transport = ReactNoopFlightServer.render( +
{}} />, + ); + 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 index 30b0863dae9..6077e44b6a6 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -218,7 +218,16 @@ 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; } } From ccb4ce432817e67ad1b33265c2d6bc43e12c5c98 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 8 Oct 2020 19:23:45 -0400 Subject: [PATCH 3/3] Error if refs are passed in server components or to client components --- .../src/__tests__/ReactFlight-test.js | 15 ++++++++++---- .../react-server/src/ReactFlightServer.js | 20 +++++++++++++++++-- scripts/error-codes/codes.json | 5 +++-- 3 files changed, 32 insertions(+), 8 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 34e03f4d7a1..865686b7e07 100644 --- 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
; } + const ref = React.createRef(); + function RefProp() { + return
; + } + const event = ReactNoopFlightServer.render(); const fn = ReactNoopFlightServer.render(); const symbol = ReactNoopFlightServer.render(); + const refs = ReactNoopFlightServer.render(); function Client({transport}) { return ReactNoopFlightClient.read(transport); @@ -185,6 +191,9 @@ describe('ReactFlight', () => { + + + , ); }); @@ -217,10 +226,8 @@ describe('ReactFlight', () => { ); }); - it('should NOT warn in DEV for key/ref getters', () => { - const transport = ReactNoopFlightServer.render( -
{}} />, - ); + it('should NOT warn in DEV for key getters', () => { + const transport = ReactNoopFlightServer.render(
); act(() => { ReactNoop.render(ReactNoopFlightClient.read(transport)); }); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6077e44b6a6..eccc17c2a19 100644 --- 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): 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): ReactModel { } } } - invariant(false, 'Unsupported type.'); + invariant( + false, + 'Unsupported server component type: %s', + describeValueForErrorMessage(type), + ); } function pingSegment(request: Request, segment: Segment): void { @@ -222,7 +235,10 @@ function isSimpleObject(object): boolean { return false; } if (!descriptor.enumerable) { - if ((names[i] === 'key' || names[i] === 'ref') && typeof descriptor.get === 'function') { + 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. diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 369c97c32a2..6eb8441d1ff 100644 --- 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." }