Skip to content
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

[Flight] Don't warn for key, but error for ref #19986

Merged
merged 3 commits into from
Oct 9, 2020
Merged
Show file tree
Hide file tree
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
16 changes: 16 additions & 0 deletions packages/react-client/src/__tests__/ReactFlight-test.js
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
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
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."
}