[Flight] Don't warn for key, but error for ref (#19986)
* 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
Sebastian Markbåge committed
Oct 8, 2020 at 20:02 UTC
0a4c7c565146222a2b701455ba2d66540063b98d
3 files changed
+71
-12
packages/react-client/src/__tests__/ReactFlight-test.js
+16
@@ -165,9 +165,15 @@ describe('ReactFlight', () => {
165
return <div foo={Symbol('foo')} />;
166
}
167
168
+ const ref = React.createRef();
169
+ function RefProp() {
170
+ return <div ref={ref} />;
171
+ }
172
+
173
const event = ReactNoopFlightServer.render(<EventHandlerProp />);
174
const fn = ReactNoopFlightServer.render(<FunctionProp />);
175
const symbol = ReactNoopFlightServer.render(<SymbolProp />);
176
+ const refs = ReactNoopFlightServer.render(<RefProp />);
177
178
function Client({transport}) {
179
return ReactNoopFlightClient.read(transport);
@@ -185,6 +191,9 @@ describe('ReactFlight', () => {
191
<ErrorBoundary expectedMessage="Symbol values (foo) cannot be passed to client components.">
192
<Client transport={symbol} />
193
</ErrorBoundary>
194
+ <ErrorBoundary expectedMessage="Refs cannot be used in server components, nor passed to client components.">
195
+ <Client transport={refs} />
196
+ </ErrorBoundary>
197
</>,
198
);
199
});
@@ -217,6 +226,13 @@ describe('ReactFlight', () => {
226
);
227
});
228
229
+ it('should NOT warn in DEV for key getters', () => {
230
+ const transport = ReactNoopFlightServer.render(<div key="a" />);
231
+ act(() => {
232
+ ReactNoop.render(ReactNoopFlightClient.read(transport));
233
+ });
234
+ });
235
+
236
it('should warn in DEV if an object with symbols is passed to a host component', () => {
237
expect(() => {
238
const transport = ReactNoopFlightServer.render(
packages/react-server/src/ReactFlightServer.js
+52
-10
@@ -119,6 +119,15 @@ export function createRequest(
119
function attemptResolveElement(element: React$Element<any>): ReactModel {
120
const type = element.type;
121
const props = element.props;
122
+ if (element.ref !== null && element.ref !== undefined) {
123
+ // When the ref moves to the regular props object this will implicitly
124
+ // throw for functions. We could probably relax it to a DEV warning for other
125
+ // cases.
126
+ invariant(
127
+ false,
128
+ 'Refs cannot be used in server components, nor passed to client components.',
129
+ );
130
+ }
131
if (typeof type === 'function') {
132
// This is a server-side component.
133
return type(props);
@@ -153,7 +162,11 @@ function attemptResolveElement(element: React$Element<any>): ReactModel {
162
}
163
}
164
}
156
- invariant(false, 'Unsupported type.');
165
+ invariant(
166
+ false,
167
+ 'Unsupported server component type: %s',
168
+ describeValueForErrorMessage(type),
169
+ );
170
}
171
172
function pingSegment(request: Request, segment: Segment): void {
@@ -218,7 +231,19 @@ function isSimpleObject(object): boolean {
231
const names = Object.getOwnPropertyNames(object);
232
for (let i = 0; i < names.length; i++) {
233
const descriptor = Object.getOwnPropertyDescriptor(object, names[i]);
221
- if (!descriptor || !descriptor.enumerable) {
234
+ if (!descriptor) {
235
+ return false;
236
+ }
237
+ if (!descriptor.enumerable) {
238
+ if (
239
+ (names[i] === 'key' || names[i] === 'ref') &&
240
+ typeof descriptor.get === 'function'
241
+ ) {
242
+ // React adds key and ref getters to props objects to issue warnings.
243
+ // Those getters will not be transferred to the client, but that's ok,
244
+ // so we'll special case them.
245
+ continue;
246
+ }
247
return false;
248
}
249
}
@@ -249,7 +274,7 @@ function describeValueForErrorMessage(value: ReactModel): string {
274
return '[...]';
275
}
276
const name = objectName(value);
252
- if (name === '[object Object]') {
277
+ if (name === 'Object') {
278
return '{...}';
279
}
280
return name;
@@ -266,6 +291,7 @@ function describeObjectForErrorMessage(
291
objectOrArray:
292
| {+[key: string | number]: ReactModel}
293
| $ReadOnlyArray<ReactModel>,
294
+ expandedName?: string,
295
): string {
296
if (isArray(objectOrArray)) {
297
let str = '[';
@@ -279,7 +305,16 @@ function describeObjectForErrorMessage(
305
str += '...';
306
break;
307
}
282
- str += describeValueForErrorMessage(array[i]);
308
+ const value = array[i];
309
+ if (
310
+ '' + i === expandedName &&
311
+ typeof value === 'object' &&
312
+ value !== null
313
+ ) {
314
+ str += describeObjectForErrorMessage(value);
315
+ } else {
316
+ str += describeValueForErrorMessage(value);
317
+ }
318
}
319
str += ']';
320
return str;
@@ -297,10 +332,17 @@ function describeObjectForErrorMessage(
332
break;
333
}
334
const name = names[i];
300
- str +=
301
- describeKeyForErrorMessage(name) +
302
- ': ' +
303
- describeValueForErrorMessage(object[name]);
335
+ str += describeKeyForErrorMessage(name) + ': ';
336
+ const value = object[name];
337
+ if (
338
+ name === expandedName &&
339
+ typeof value === 'object' &&
340
+ value !== null
341
+ ) {
342
+ str += describeObjectForErrorMessage(value);
343
+ } else {
344
+ str += describeValueForErrorMessage(value);
345
+ }
346
}
347
str += '}';
348
return str;
@@ -444,7 +486,7 @@ export function resolveModelToJSON(
486
'Classes or other objects with methods are not supported. ' +
487
'Remove %s from these props: %s',
488
describeKeyForErrorMessage(key),
447
- describeObjectForErrorMessage(parent),
489
+ describeObjectForErrorMessage(parent, key),
490
);
491
} else if (Object.getOwnPropertySymbols) {
492
const symbols = Object.getOwnPropertySymbols(value);
@@ -455,7 +497,7 @@ export function resolveModelToJSON(
497
'Remove %s from these props: %s',
498
symbols[0].description,
499
describeKeyForErrorMessage(key),
458
- describeObjectForErrorMessage(parent),
500
+ describeObjectForErrorMessage(parent, key),
501
);
502
}
503
}
scripts/error-codes/codes.json
+3
-2
@@ -340,7 +340,7 @@
340
"348": "ensureListeningTo(): received a container that was not an element node. This is likely a bug in React.",
341
"349": "Expected a work-in-progress root. This is a bug in React. Please file an issue.",
342
"350": "Cannot read from mutable source during the current render without tearing. This is a bug in React. Please file an issue.",
343
- "351": "Unsupported type.",
343
+ "351": "Unsupported server component type: %s",
344
"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.",
345
"353": "A server block should never encode any other slots. This is a bug in React.",
346
"354": "getInspectorDataForViewAtPoint() is not available in production.",
@@ -366,5 +366,6 @@
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"
369
+ "378": "Type %s is not supported in client component props. Remove %s from this object, or avoid the entire object: %s",
370
+ "379": "Refs cannot be used in server components, nor passed to client components."
371
}