Skip to content

Commit 29bc544

Browse files
committed
Improve debugging DX when string coercions fail
If a coercion throws, the check function will now throw after it calls console.error(). It's the same exception that will be thrown after the check function returns, but by throwing it inside of the check function, it will be easier for callers to understand what happened and how to fix it. I added a bunch of comments above where it throws, so that callers who follow links from a call stack will see troubleshooting info. This commit also fixes a mistake where one block of DEV-only code wasn't included in an `if (__DEV__) {}` block.
1 parent f7a0e55 commit 29bc544

File tree

1 file changed

+43
-18
lines changed

1 file changed

+43
-18
lines changed

packages/shared/CheckStringCoercion.js

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,33 +34,52 @@ function typeName(value: mixed): string {
3434

3535
// $FlowFixMe only called in DEV, so void return is not possible.
3636
function willCoercionThrow(value: mixed): boolean {
37-
if (
38-
value !== null &&
39-
typeof value === 'object' &&
40-
value.$$typeof === REACT_OPAQUE_ID_TYPE
41-
) {
42-
// OpaqueID type is expected to throw, so React will handle it. Not sure if
43-
// it's expected that string coercion will throw, but we'll assume it's OK.
44-
// See https://github.com/facebook/react/issues/20127.
45-
return false;
46-
}
4737
if (__DEV__) {
38+
if (
39+
value !== null &&
40+
typeof value === 'object' &&
41+
value.$$typeof === REACT_OPAQUE_ID_TYPE
42+
) {
43+
// OpaqueID type is expected to throw, so React will handle it. Not sure if
44+
// it's expected that string coercion will throw, but we'll assume it's OK.
45+
// See https://github.com/facebook/react/issues/20127.
46+
return;
47+
}
4848
try {
49-
// eslint-disable-next-line react-internal/safe-string-coercion
50-
void ('' + (value: any));
49+
testStringCoercion(value);
5150
return false;
5251
} catch (e) {
5352
return true;
5453
}
5554
}
5655
}
5756

58-
// TODO: what is React's "dev-only invariant"? I don't only want to show a
59-
// console error because the next line after this returns will crash, so the
60-
// whole point of these functions is to crash here, not after it returns. The
61-
// invariant implementation does neat things with exception handling in DEV. But
62-
// the lint message from invariant sounds like it will have a prod footprint,
63-
// but this is dev-only code. What to do?
57+
function testStringCoercion(value: mixed) {
58+
// If you ended up here by following an exception call stack, here's what's
59+
// happened: you supplied an object or symbol value to React (as a prop, key,
60+
// DOM attribute, CSS property, string ref, etc.) and when React tried to
61+
// coerce it to a string using `'' + value`, an exception was thrown.
62+
//
63+
// The most common types that will cause this exception are `Symbol` instances
64+
// and Temporal objects like `Temporal.Instant`. But any object that has a
65+
// `valueOf` or `[Symbol.toPrimitive]` method that throws will also cause this
66+
// exception. (Library authors do this to prevent users from using built-in
67+
// numeric operators like `+` or comparison operators like `>=` because custom
68+
// methods are needed to perform accurate arithmetic or comparison.)
69+
//
70+
// To fix the problem, coerce this object or symbol value to a string before
71+
// passing it to React. The most reliable way is usually `String(value)`.
72+
//
73+
// To find which value is throwing, check the browser or debugger console.
74+
// Before this exception was thrown, there should be `console.error` output
75+
// that shows the type (Symbol, Temporal.PlainDate, etc.) that caused the
76+
// problem and how that type was used: key, atrribute, input value prop, etc.
77+
// In most cases, this console output also shows the component and its
78+
// ancestor components where the exception happened.
79+
//
80+
// eslint-disable-next-line react-internal/safe-string-coercion
81+
return '' + (value: any);
82+
}
6483

6584
export function checkAttributeStringCoercion(
6685
value: mixed,
@@ -74,6 +93,7 @@ export function checkAttributeStringCoercion(
7493
attributeName,
7594
typeName(value),
7695
);
96+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
7797
}
7898
}
7999
}
@@ -86,6 +106,7 @@ export function checkKeyStringCoercion(value: mixed) {
86106
' This value must be coerced to a string before before using it here.',
87107
typeName(value),
88108
);
109+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
89110
}
90111
}
91112
}
@@ -99,6 +120,7 @@ export function checkPropStringCoercion(value: mixed, propName: string) {
99120
propName,
100121
typeName(value),
101122
);
123+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
102124
}
103125
}
104126
}
@@ -112,6 +134,7 @@ export function checkCSSPropertyStringCoercion(value: mixed, propName: string) {
112134
propName,
113135
typeName(value),
114136
);
137+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
115138
}
116139
}
117140
}
@@ -124,6 +147,7 @@ export function checkHtmlStringCoercion(value: mixed) {
124147
' This value must be coerced to a string before before using it here.',
125148
typeName(value),
126149
);
150+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
127151
}
128152
}
129153
}
@@ -137,6 +161,7 @@ export function checkFormFieldValueStringCoercion(value: mixed) {
137161
' This value must be coerced to a string before before using it here.',
138162
typeName(value),
139163
);
164+
return testStringCoercion(value); // throw (to help callers find troubleshooting comments)
140165
}
141166
}
142167
}

0 commit comments

Comments
 (0)