Skip to content

Commit

Permalink
[Flight] Don't warn for key, but error for ref (#19986)
Browse files Browse the repository at this point in the history 
* Improve error message by expanding the object in question

* Don't warn for key/ref getters

* Error if refs are passed in server components or to client components
  • Loading branch information
@sebmarkbage
sebmarkbage authored Oct 9, 2020
1 parent 993ca53 commit 0a4c7c5
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 12 deletions.
16 changes: 16 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,15 @@ describe('ReactFlight', () => {
return <div foo={Symbol('foo')} />;
}

const ref = React.createRef();
function RefProp() {
return <div ref={ref} />;
}

const event = ReactNoopFlightServer.render(<EventHandlerProp />);
const fn = ReactNoopFlightServer.render(<FunctionProp />);
const symbol = ReactNoopFlightServer.render(<SymbolProp />);
const refs = ReactNoopFlightServer.render(<RefProp />);

function Client({transport}) {
return ReactNoopFlightClient.read(transport);
Expand All @@ -185,6 +191,9 @@ describe('ReactFlight', () => {
<ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
<Client transport={symbol} />
</ErrorBoundary>
<ErrorBoundary expectedMessage="Refs cannot be used in server components, nor passed to client components.">
<Client transport={refs} />
</ErrorBoundary>
</>,
);
});
Expand Down Expand Up @@ -217,6 +226,13 @@ describe('ReactFlight', () => {
);
});

it('should NOT warn in DEV for key getters', () => {
const transport = ReactNoopFlightServer.render(<div key="a" />);
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(
Expand Down
62 changes: 52 additions & 10 deletions packages/react-server/src/ReactFlightServer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ export function createRequest(
function attemptResolveElement(element: React$Element<any>): 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);
Expand Down Expand Up @@ -153,7 +162,11 @@ function attemptResolveElement(element: React$Element<any>): ReactModel {
}
}
}
invariant(false, 'Unsupported type.');
invariant(
false,
'Unsupported server component type: %s',
describeValueForErrorMessage(type),
);
}

function pingSegment(request: Request, segment: Segment): void {
Expand Down Expand Up @@ -218,7 +231,19 @@ 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;
}
}
Expand Down Expand Up @@ -249,7 +274,7 @@ function describeValueForErrorMessage(value: ReactModel): string {
return '[...]';
}
const name = objectName(value);
if (name === '[object Object]') {
if (name === 'Object') {
return '{...}';
}
return name;
Expand All @@ -266,6 +291,7 @@ function describeObjectForErrorMessage(
objectOrArray:
| {+[key: string | number]: ReactModel}
| $ReadOnlyArray<ReactModel>,
expandedName?: string,
): string {
if (isArray(objectOrArray)) {
let str = '[';
Expand All @@ -279,7 +305,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;
Expand All @@ -297,10 +332,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;
Expand Down Expand Up @@ -444,7 +486,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);
Expand All @@ -455,7 +497,7 @@ export function resolveModelToJSON(
'Remove %s from these props: %s',
symbols[0].description,
describeKeyForErrorMessage(key),
describeObjectForErrorMessage(parent),
describeObjectForErrorMessage(parent, key),
);
}
}
Expand Down
5 changes: 3 additions & 2 deletions scripts/error-codes/codes.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand All @@ -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."
}

0 comments on commit 0a4c7c5

Please sign in to comment.