Encode better error messages when part of the context is a client reference (#28355)
Alternative to #28354. If a client reference is one of the props being describes as part of another error, we call toString on it, which errors. We should error explicitly when a Symbol prop is extracted. However, pragmatically I added the toString symbol tag even though we don't know what the real tostring will be but we also lie about the typeof. We can however in addition to this give it a different description because describing this property as an object isn't quite right. We probably could extract the export name but that's kind of renderer specific and I just added this shared module to Fizz which doesn't have that which is unfortunate an consequence. For default exports we don't have a good name of what the alias was in the receiver. Could maybe call it "default" but for now I just call it "client".
Sebastian Markbåge committed
Feb 19, 2024 at 11:49 UTC
c323f8290c2336713b62c69322bb30ed63234c24
4 files changed
+230
-97
packages/react-server-dom-turbopack/src/ReactFlightTurbopackReferences.js
+129
-93
@@ -92,7 +92,11 @@ export function registerServerReference<T: Function>(
92
const PROMISE_PROTOTYPE = Promise.prototype;
93
94
const deepProxyHandlers = {
95
- get: function (target: Function, name: string, receiver: Proxy<Function>) {
95
+ get: function (
96
+ target: Function,
97
+ name: string | symbol,
98
+ receiver: Proxy<Function>,
99
+ ) {
100
switch (name) {
101
// These names are read by the Flight runtime if you end up using the exports object.
102
case '$$typeof':
@@ -117,6 +121,9 @@ const deepProxyHandlers = {
121
case Symbol.toPrimitive:
122
// $FlowFixMe[prop-missing]
123
return Object.prototype[Symbol.toPrimitive];
124
+ case Symbol.toStringTag:
125
+ // $FlowFixMe[prop-missing]
126
+ return Object.prototype[Symbol.toStringTag];
127
case 'Provider':
128
throw new Error(
129
`Cannot render a Client Context Provider on the Server. ` +
@@ -137,105 +144,134 @@ const deepProxyHandlers = {
144
},
145
};
146
140
-const proxyHandlers = {
141
- get: function (
142
- target: Function,
143
- name: string,
144
- receiver: Proxy<Function>,
145
- ): $FlowFixMe {
146
- switch (name) {
147
- // These names are read by the Flight runtime if you end up using the exports object.
148
- case '$$typeof':
149
- return target.$$typeof;
150
- case '$$id':
151
- return target.$$id;
152
- case '$$async':
153
- return target.$$async;
154
- case 'name':
155
- return target.name;
156
- // We need to special case this because createElement reads it if we pass this
157
- // reference.
158
- case 'defaultProps':
159
- return undefined;
160
- // Avoid this attempting to be serialized.
161
- case 'toJSON':
162
- return undefined;
163
- case Symbol.toPrimitive:
164
- // $FlowFixMe[prop-missing]
165
- return Object.prototype[Symbol.toPrimitive];
166
- case '__esModule':
167
- // Something is conditionally checking which export to use. We'll pretend to be
168
- // an ESM compat module but then we'll check again on the client.
169
- const moduleId = target.$$id;
170
- target.default = registerClientReferenceImpl(
171
- (function () {
172
- throw new Error(
173
- `Attempted to call the default export of ${moduleId} from the server ` +
174
- `but it's on the client. It's not possible to invoke a client function from ` +
175
- `the server, it can only be rendered as a Component or passed to props of a ` +
176
- `Client Component.`,
177
- );
178
- }: any),
179
- target.$$id + '#',
180
- target.$$async,
181
- );
182
- return true;
183
- case 'then':
184
- if (target.then) {
185
- // Use a cached value
186
- return target.then;
187
- }
188
- if (!target.$$async) {
189
- // If this module is expected to return a Promise (such as an AsyncModule) then
190
- // we should resolve that with a client reference that unwraps the Promise on
191
- // the client.
192
-
193
- const clientReference: ClientReference<any> =
194
- registerClientReferenceImpl(({}: any), target.$$id, true);
195
- const proxy = new Proxy(clientReference, proxyHandlers);
196
-
197
- // Treat this as a resolved Promise for React's use()
198
- target.status = 'fulfilled';
199
- target.value = proxy;
200
-
201
- const then = (target.then = registerClientReferenceImpl(
202
- (function then(resolve, reject: any) {
203
- // Expose to React.
204
- return Promise.resolve(resolve(proxy));
205
- }: any),
206
- // If this is not used as a Promise but is treated as a reference to a `.then`
207
- // export then we should treat it as a reference to that name.
208
- target.$$id + '#then',
209
- false,
210
- ));
211
- return then;
212
- } else {
213
- // Since typeof .then === 'function' is a feature test we'd continue recursing
214
- // indefinitely if we return a function. Instead, we return an object reference
215
- // if we check further.
216
- return undefined;
217
- }
218
- }
219
- let cachedReference = target[name];
220
- if (!cachedReference) {
221
- const reference: ClientReference<any> = registerClientReferenceImpl(
147
+function getReference(target: Function, name: string | symbol): $FlowFixMe {
148
+ switch (name) {
149
+ // These names are read by the Flight runtime if you end up using the exports object.
150
+ case '$$typeof':
151
+ return target.$$typeof;
152
+ case '$$id':
153
+ return target.$$id;
154
+ case '$$async':
155
+ return target.$$async;
156
+ case 'name':
157
+ return target.name;
158
+ // We need to special case this because createElement reads it if we pass this
159
+ // reference.
160
+ case 'defaultProps':
161
+ return undefined;
162
+ // Avoid this attempting to be serialized.
163
+ case 'toJSON':
164
+ return undefined;
165
+ case Symbol.toPrimitive:
166
+ // $FlowFixMe[prop-missing]
167
+ return Object.prototype[Symbol.toPrimitive];
168
+ case Symbol.toStringTag:
169
+ // $FlowFixMe[prop-missing]
170
+ return Object.prototype[Symbol.toStringTag];
171
+ case '__esModule':
172
+ // Something is conditionally checking which export to use. We'll pretend to be
173
+ // an ESM compat module but then we'll check again on the client.
174
+ const moduleId = target.$$id;
175
+ target.default = registerClientReferenceImpl(
176
(function () {
177
throw new Error(
224
- // eslint-disable-next-line react-internal/safe-string-coercion
225
- `Attempted to call ${String(name)}() from the server but ${String(
226
- name,
227
- )} is on the client. ` +
228
- `It's not possible to invoke a client function from the server, it can ` +
229
- `only be rendered as a Component or passed to props of a Client Component.`,
178
+ `Attempted to call the default export of ${moduleId} from the server ` +
179
+ `but it's on the client. It's not possible to invoke a client function from ` +
180
+ `the server, it can only be rendered as a Component or passed to props of a ` +
181
+ `Client Component.`,
182
);
183
}: any),
232
- target.$$id + '#' + name,
184
+ target.$$id + '#',
185
target.$$async,
186
);
235
- Object.defineProperty((reference: any), 'name', {value: name});
236
- cachedReference = target[name] = new Proxy(reference, deepProxyHandlers);
187
+ return true;
188
+ case 'then':
189
+ if (target.then) {
190
+ // Use a cached value
191
+ return target.then;
192
+ }
193
+ if (!target.$$async) {
194
+ // If this module is expected to return a Promise (such as an AsyncModule) then
195
+ // we should resolve that with a client reference that unwraps the Promise on
196
+ // the client.
197
+
198
+ const clientReference: ClientReference<any> =
199
+ registerClientReferenceImpl(({}: any), target.$$id, true);
200
+ const proxy = new Proxy(clientReference, proxyHandlers);
201
+
202
+ // Treat this as a resolved Promise for React's use()
203
+ target.status = 'fulfilled';
204
+ target.value = proxy;
205
+
206
+ const then = (target.then = registerClientReferenceImpl(
207
+ (function then(resolve, reject: any) {
208
+ // Expose to React.
209
+ return Promise.resolve(resolve(proxy));
210
+ }: any),
211
+ // If this is not used as a Promise but is treated as a reference to a `.then`
212
+ // export then we should treat it as a reference to that name.
213
+ target.$$id + '#then',
214
+ false,
215
+ ));
216
+ return then;
217
+ } else {
218
+ // Since typeof .then === 'function' is a feature test we'd continue recursing
219
+ // indefinitely if we return a function. Instead, we return an object reference
220
+ // if we check further.
221
+ return undefined;
222
+ }
223
+ }
224
+ if (typeof name === 'symbol') {
225
+ throw new Error(
226
+ 'Cannot read Symbol exports. Only named exports are supported on a client module ' +
227
+ 'imported on the server.',
228
+ );
229
+ }
230
+ let cachedReference = target[name];
231
+ if (!cachedReference) {
232
+ const reference: ClientReference<any> = registerClientReferenceImpl(
233
+ (function () {
234
+ throw new Error(
235
+ // eslint-disable-next-line react-internal/safe-string-coercion
236
+ `Attempted to call ${String(name)}() from the server but ${String(
237
+ name,
238
+ )} is on the client. ` +
239
+ `It's not possible to invoke a client function from the server, it can ` +
240
+ `only be rendered as a Component or passed to props of a Client Component.`,
241
+ );
242
+ }: any),
243
+ target.$$id + '#' + name,
244
+ target.$$async,
245
+ );
246
+ Object.defineProperty((reference: any), 'name', {value: name});
247
+ cachedReference = target[name] = new Proxy(reference, deepProxyHandlers);
248
+ }
249
+ return cachedReference;
250
+}
251
+
252
+const proxyHandlers = {
253
+ get: function (
254
+ target: Function,
255
+ name: string | symbol,
256
+ receiver: Proxy<Function>,
257
+ ): $FlowFixMe {
258
+ return getReference(target, name);
259
+ },
260
+ getOwnPropertyDescriptor: function (
261
+ target: Function,
262
+ name: string | symbol,
263
+ ): $FlowFixMe {
264
+ let descriptor = Object.getOwnPropertyDescriptor(target, name);
265
+ if (!descriptor) {
266
+ descriptor = {
267
+ value: getReference(target, name),
268
+ writable: false,
269
+ configurable: false,
270
+ enumerable: false,
271
+ };
272
+ Object.defineProperty(target, name, descriptor);
273
}
238
- return cachedReference;
274
+ return descriptor;
275
},
276
getPrototypeOf(target: Function): Object {
277
// Pretend to be a Promise in case anyone asks.
packages/react-server-dom-webpack/src/ReactFlightWebpackReferences.js
+20
-4
@@ -92,7 +92,11 @@ export function registerServerReference<T: Function>(
92
const PROMISE_PROTOTYPE = Promise.prototype;
93
94
const deepProxyHandlers = {
95
- get: function (target: Function, name: string, receiver: Proxy<Function>) {
95
+ get: function (
96
+ target: Function,
97
+ name: string | symbol,
98
+ receiver: Proxy<Function>,
99
+ ) {
100
switch (name) {
101
// These names are read by the Flight runtime if you end up using the exports object.
102
case '$$typeof':
@@ -117,6 +121,9 @@ const deepProxyHandlers = {
121
case Symbol.toPrimitive:
122
// $FlowFixMe[prop-missing]
123
return Object.prototype[Symbol.toPrimitive];
124
+ case Symbol.toStringTag:
125
+ // $FlowFixMe[prop-missing]
126
+ return Object.prototype[Symbol.toStringTag];
127
case 'Provider':
128
throw new Error(
129
`Cannot render a Client Context Provider on the Server. ` +
@@ -137,7 +144,7 @@ const deepProxyHandlers = {
144
},
145
};
146
140
-function getReference(target: Function, name: string): $FlowFixMe {
147
+function getReference(target: Function, name: string | symbol): $FlowFixMe {
148
switch (name) {
149
// These names are read by the Flight runtime if you end up using the exports object.
150
case '$$typeof':
@@ -158,6 +165,9 @@ function getReference(target: Function, name: string): $FlowFixMe {
165
case Symbol.toPrimitive:
166
// $FlowFixMe[prop-missing]
167
return Object.prototype[Symbol.toPrimitive];
168
+ case Symbol.toStringTag:
169
+ // $FlowFixMe[prop-missing]
170
+ return Object.prototype[Symbol.toStringTag];
171
case '__esModule':
172
// Something is conditionally checking which export to use. We'll pretend to be
173
// an ESM compat module but then we'll check again on the client.
@@ -211,6 +221,12 @@ function getReference(target: Function, name: string): $FlowFixMe {
221
return undefined;
222
}
223
}
224
+ if (typeof name === 'symbol') {
225
+ throw new Error(
226
+ 'Cannot read Symbol exports. Only named exports are supported on a client module ' +
227
+ 'imported on the server.',
228
+ );
229
+ }
230
let cachedReference = target[name];
231
if (!cachedReference) {
232
const reference: ClientReference<any> = registerClientReferenceImpl(
@@ -236,14 +252,14 @@ function getReference(target: Function, name: string): $FlowFixMe {
252
const proxyHandlers = {
253
get: function (
254
target: Function,
239
- name: string,
255
+ name: string | symbol,
256
receiver: Proxy<Function>,
257
): $FlowFixMe {
258
return getReference(target, name);
259
},
260
getOwnPropertyDescriptor: function (
261
target: Function,
246
- name: string,
262
+ name: string | symbol,
263
): $FlowFixMe {
264
let descriptor = Object.getOwnPropertyDescriptor(target, name);
265
if (!descriptor) {
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOM-test.js
+67
@@ -568,6 +568,32 @@ describe('ReactFlightDOM', () => {
568
);
569
});
570
571
+ it('throws when accessing a symbol prop from client exports', () => {
572
+ const symbol = Symbol('test');
573
+ const ClientModule = clientExports({
574
+ Component: {deep: 'thing'},
575
+ });
576
+ function read() {
577
+ return ClientModule[symbol];
578
+ }
579
+ expect(read).toThrowError(
580
+ 'Cannot read Symbol exports. ' +
581
+ 'Only named exports are supported on a client module imported on the server.',
582
+ );
583
+ });
584
+
585
+ it('does not throw when toString:ing client exports', () => {
586
+ const ClientModule = clientExports({
587
+ Component: {deep: 'thing'},
588
+ });
589
+ expect(Object.prototype.toString.call(ClientModule)).toBe(
590
+ '[object Object]',
591
+ );
592
+ expect(Object.prototype.toString.call(ClientModule.Component)).toBe(
593
+ '[object Function]',
594
+ );
595
+ });
596
+
597
it('does not throw when React inspects any deep props', () => {
598
const ClientModule = clientExports({
599
Component: function () {},
@@ -1655,4 +1681,45 @@ describe('ReactFlightDOM', () => {
1681
await collectHints(readable);
1682
expect(hintRows.length).toEqual(6);
1683
});
1684
+
1685
+ it('should be able to include a client reference in printed errors', async () => {
1686
+ const reportedErrors = [];
1687
+
1688
+ const ClientComponent = clientExports(function ({prop}) {
1689
+ return 'This should never render';
1690
+ });
1691
+
1692
+ const ClientReference = clientExports({});
1693
+
1694
+ class InvalidValue {}
1695
+
1696
+ const {writable} = getTestStream();
1697
+ const {pipe} = ReactServerDOMServer.renderToPipeableStream(
1698
+ <div>
1699
+ <ClientComponent prop={ClientReference} invalid={InvalidValue} />
1700
+ </div>,
1701
+ webpackMap,
1702
+ {
1703
+ onError(x) {
1704
+ reportedErrors.push(x);
1705
+ },
1706
+ },
1707
+ );
1708
+ pipe(writable);
1709
+
1710
+ expect(reportedErrors.length).toBe(1);
1711
+ if (__DEV__) {
1712
+ expect(reportedErrors[0].message).toEqual(
1713
+ 'Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".\n' +
1714
+ ' <... prop={client} invalid={function}>\n' +
1715
+ ' ^^^^^^^^^^',
1716
+ );
1717
+ } else {
1718
+ expect(reportedErrors[0].message).toEqual(
1719
+ 'Functions cannot be passed directly to Client Components unless you explicitly expose it by marking it with "use server".\n' +
1720
+ ' {prop: client, invalid: function}\n' +
1721
+ ' ^^^^^^^^',
1722
+ );
1723
+ }
1724
+ });
1725
});
packages/shared/ReactSerializationErrors.js
+14
@@ -98,6 +98,9 @@ export function describeValueForErrorMessage(value: mixed): string {
98
if (isArray(value)) {
99
return '[...]';
100
}
101
+ if (value !== null && value.$$typeof === CLIENT_REFERENCE_TAG) {
102
+ return describeClientReference(value);
103
+ }
104
const name = objectName(value);
105
if (name === 'Object') {
106
return '{...}';
@@ -105,6 +108,9 @@ export function describeValueForErrorMessage(value: mixed): string {
108
return name;
109
}
110
case 'function':
111
+ if ((value: any).$$typeof === CLIENT_REFERENCE_TAG) {
112
+ return describeClientReference(value);
113
+ }
114
return 'function';
115
default:
116
// eslint-disable-next-line react-internal/safe-string-coercion
@@ -142,6 +148,12 @@ function describeElementType(type: any): string {
148
return '';
149
}
150
151
+const CLIENT_REFERENCE_TAG = Symbol.for('react.client.reference');
152
+
153
+function describeClientReference(ref: any) {
154
+ return 'client';
155
+}
156
+
157
export function describeObjectForErrorMessage(
158
objectOrArray: {+[key: string | number]: mixed, ...} | $ReadOnlyArray<mixed>,
159
expandedName?: string,
@@ -210,6 +222,8 @@ export function describeObjectForErrorMessage(
222
} else {
223
if (objectOrArray.$$typeof === REACT_ELEMENT_TYPE) {
224
str = '<' + describeElementType(objectOrArray.type) + '/>';
225
+ } else if (objectOrArray.$$typeof === CLIENT_REFERENCE_TAG) {
226
+ return describeClientReference(objectOrArray);
227
} else if (__DEV__ && jsxPropsParents.has(objectOrArray)) {
228
// Print JSX
229
const type = jsxPropsParents.get(objectOrArray);