Improve DEV errors if string coercion throws (Temporal.*, Symbol, etc.)

.eslintrc.js

@@ -108,6 +108,10 @@ module.exports = {
     // CUSTOM RULES
     // the second argument of warning/invariant should be a literal string
     'react-internal/no-primitive-constructors': ERROR,
+    'react-internal/safe-string-coercion': [
+      ERROR,
+      {isProductionUserAppCode: true},
+    ],
     'react-internal/no-to-warn-dev-within-to-throw': ERROR,
     'react-internal/invariant-args': ERROR,
     'react-internal/warning-args': ERROR,
@@ -168,10 +172,17 @@ module.exports = {
         'packages/*/npm/**/*.js',
         'packages/dom-event-testing-library/**/*.js',
         'packages/react-devtools*/**/*.js',
+        'dangerfile.js',
+        'fixtures',
+        'packages/react-dom/src/test-utils/*.js',
       ],
       rules: {
         'react-internal/no-production-logging': OFF,
         'react-internal/warning-args': OFF,
+        'react-internal/safe-string-coercion': [
+          ERROR,
+          {isProductionUserAppCode: false},
+        ],
 
         // Disable accessibility checks
         'jsx-a11y/aria-role': OFF,
@@ -185,7 +196,7 @@ module.exports = {
     {
       files: [
         'scripts/eslint-rules/*.js',
-        'packages/eslint-plugin-react-hooks/src/*.js'
+        'packages/eslint-plugin-react-hooks/src/*.js',
       ],
       plugins: ['eslint-plugin'],
       rules: {

dangerfile.js

@@ -102,8 +102,8 @@ function row(result) {
   let headSha;
   let baseSha;
   try {
-    headSha = (readFileSync(HEAD_DIR + '/COMMIT_SHA') + '').trim();
-    baseSha = (readFileSync(BASE_DIR + '/COMMIT_SHA') + '').trim();
+    headSha = String(readFileSync(HEAD_DIR + '/COMMIT_SHA')).trim();
+    baseSha = String(readFileSync(BASE_DIR + '/COMMIT_SHA')).trim();
   } catch {
     warn(
       "Failed to read build artifacts. It's possible a build configuration " +

fixtures/dom/src/components/fixtures/error-handling/index.js

@@ -41,7 +41,7 @@ class ErrorBoundary extends React.Component {
       if (this.state.error) {
         return <p>Captured an error: {this.state.error.message}</p>;
       } else {
-        return <p>Captured an error: {'' + this.state.error}</p>;
+        return <p>Captured an error: {String(this.state.error)}</p>;
       }
     }
     if (this.state.shouldThrow) {

packages/react-debug-tools/src/__tests__/ReactHooksInspectionIntegration-test.js

@@ -615,7 +615,7 @@ describe('ReactHooksInspectionIntegration', () => {
     expect(tree[0].id).toEqual(0);
     expect(tree[0].isStateEditable).toEqual(false);
     expect(tree[0].name).toEqual('OpaqueIdentifier');
-    expect((tree[0].value + '').startsWith('c_')).toBe(true);
+    expect(String(tree[0].value).startsWith('c_')).toBe(true);
 
     expect(tree[1]).toEqual({
       id: 1,
@@ -646,7 +646,7 @@ describe('ReactHooksInspectionIntegration', () => {
     expect(tree[0].id).toEqual(0);
     expect(tree[0].isStateEditable).toEqual(false);
     expect(tree[0].name).toEqual('OpaqueIdentifier');
-    expect((tree[0].value + '').startsWith('c_')).toBe(true);
+    expect(String(tree[0].value).startsWith('c_')).toBe(true);
 
     expect(tree[1]).toEqual({
       id: 1,

packages/react-devtools-extensions/src/main.js

@@ -94,7 +94,7 @@ function createPanelIfReactLoaded() {
 
       function initBridgeAndStore() {
         const port = chrome.runtime.connect({
-          name: '' + tabId,
+          name: String(tabId),
         });
         // Looks like `port.onDisconnect` does not trigger on in-tab navigation like new URL or back/forward navigation,
         // so it makes no sense to handle it here.

packages/react-devtools-shared/src/backend/legacy/renderer.js

@@ -63,7 +63,7 @@ function getData(internalInstance: InternalInstance) {
   // != used deliberately here to catch undefined and null
   if (internalInstance._currentElement != null) {
     if (internalInstance._currentElement.key) {
-      key = '' + internalInstance._currentElement.key;
+      key = String(internalInstance._currentElement.key);
     }
 
     const elementType = internalInstance._currentElement.type;

packages/react-devtools-shared/src/backend/renderer.js

@@ -1847,7 +1847,7 @@ export function attach(
 
       // This check is a guard to handle a React element that has been modified
       // in such a way as to bypass the default stringification of the "key" property.
-      const keyString = key === null ? null : '' + key;
+      const keyString = key === null ? null : String(key);
       const keyStringID = getStringID(keyString);
 
       pushOperation(TREE_OPERATION_ADD);

packages/react-devtools-shared/src/backend/utils.js

@@ -166,11 +166,7 @@ export function format(
 ): string {
   const args = inputArgs.slice();
 
-  // Symbols cannot be concatenated with Strings.
-  let formatted: string =
-    typeof maybeMessage === 'symbol'
-      ? maybeMessage.toString()
-      : '' + maybeMessage;
+  let formatted: string = String(maybeMessage);
 
   // If the first argument is a string, check for substitutions.
   if (typeof maybeMessage === 'string') {
@@ -203,17 +199,14 @@ export function format(
   // Arguments that remain after formatting.
   if (args.length) {
     for (let i = 0; i < args.length; i++) {
-      const arg = args[i];
-
-      // Symbols cannot be concatenated with Strings.
-      formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
+      formatted += ' ' + String(args[i]);
     }
   }
 
   // Update escaped %% values.
   formatted = formatted.replace(/%{2,2}/g, '%');
 
-  return '' + formatted;
+  return String(formatted);
 }
 
 export function isSynchronousXHRSupported(): boolean {

packages/react-devtools-shared/src/devtools/views/ErrorBoundary/ErrorBoundary.js

@@ -46,7 +46,7 @@ export default class ErrorBoundary extends Component<Props, State> {
       error !== null &&
       error.hasOwnProperty('message')
         ? error.message
-        : '' + error;
+        : String(error);
 
     const callStack =
       typeof error === 'object' &&

packages/react-devtools-shared/src/devtools/views/utils.js

@@ -19,8 +19,8 @@ export function alphaSortEntries(
 ): number {
   const a = entryA[0];
   const b = entryB[0];
-  if ('' + +a === a) {
-    if ('' + +b !== b) {
+  if (String(+a) === a) {
+    if (String(+b) !== b) {
       return -1;
     }
     return +a < +b ? -1 : 1;

packages/react-devtools-shared/src/hook.js

@@ -180,10 +180,7 @@ export function installHook(target: any): DevToolsHook | null {
     const args = inputArgs.slice();
 
     // Symbols cannot be concatenated with Strings.
-    let formatted: string =
-      typeof maybeMessage === 'symbol'
-        ? maybeMessage.toString()
-        : '' + maybeMessage;
+    let formatted = String(maybeMessage);
 
     // If the first argument is a string, check for substitutions.
     if (typeof maybeMessage === 'string') {
@@ -216,17 +213,14 @@ export function installHook(target: any): DevToolsHook | null {
     // Arguments that remain after formatting.
     if (args.length) {
       for (let i = 0; i < args.length; i++) {
-        const arg = args[i];
-
-        // Symbols cannot be concatenated with Strings.
-        formatted += ' ' + (typeof arg === 'symbol' ? arg.toString() : arg);
+        formatted += ' ' + String(args[i]);
       }
     }
 
     // Update escaped %% values.
     formatted = formatted.replace(/%{2,2}/g, '%');
 
-    return '' + formatted;
+    return String(formatted);
   }
 
   let unpatchFn = null;

packages/react-devtools-shared/src/hooks/SourceMapMetadataConsumer.js

@@ -38,7 +38,6 @@ function normalizeSourcePath(
   const {sourceRoot} = map;
   let source = sourceInput;
 
-  // eslint-disable-next-line react-internal/no-primitive-constructors
   source = String(source);
   // Some source maps produce relative source paths like "./foo.js" instead of
   // "foo.js".  Normalize these first so that future comparisons will succeed.

packages/react-devtools-shared/src/utils.js

@@ -834,7 +834,7 @@ export function formatDataForPreview(
       return data;
     default:
       try {
-        return truncateForDisplay('' + data);
+        return truncateForDisplay(String(data));
       } catch (error) {
         return 'unserializable';
       }

packages/react-dom/src/__tests__/ReactDOMAttribute-test.js

@@ -89,6 +89,27 @@ describe('ReactDOM unknown attribute', () => {
       testUnknownAttributeAssignment(lol, 'lol');
     });
 
+    it('throws with Temporal-like objects', () => {
+      class TemporalLike {
+        valueOf() {
+          // Throwing here is the behavior of ECMAScript "Temporal" date/time API.
+          // See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
+          throw new TypeError('prod message');
+        }
+        toString() {
+          return '2020-01-01';
+        }
+      }
+      const test = () =>
+        testUnknownAttributeAssignment(new TemporalLike(), null);
+      expect(() =>
+        expect(test).toThrowError(new TypeError('prod message')),
+      ).toErrorDev(
+        'Warning: The provided `unknown` attribute is an unsupported type TemporalLike.' +
+          ' This value must be coerced to a string before before using it here.',
+      );
+    });
+
     it('removes symbols and warns', () => {
       expect(() => testUnknownAttributeRemoval(Symbol('foo'))).toErrorDev(
         'Warning: Invalid value for prop `unknown` on <div> tag. Either remove it ' +

packages/react-dom/src/__tests__/ReactDOMComponent-test.js

@@ -254,6 +254,28 @@ describe('ReactDOMComponent', () => {
       ReactDOM.render(<span style={style} />, div);
     });
 
+    it('throws with Temporal-like objects as style values', () => {
+      class TemporalLike {
+        valueOf() {
+          // Throwing here is the behavior of ECMAScript "Temporal" date/time API.
+          // See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
+          throw new TypeError('prod message');
+        }
+        toString() {
+          return '2020-01-01';
+        }
+      }
+      const style = {fontSize: new TemporalLike()};
+      const div = document.createElement('div');
+      const test = () => ReactDOM.render(<span style={style} />, div);
+      expect(() =>
+        expect(test).toThrowError(new TypeError('prod message')),
+      ).toErrorDev(
+        'Warning: The provided `fontSize` CSS property is an unsupported type TemporalLike.' +
+          ' This value must be coerced to a string before before using it here.',
+      );
+    });
+
     it('should update styles if initially null', () => {
       let styles = null;
       const container = document.createElement('div');
@@ -1130,7 +1152,7 @@ describe('ReactDOMComponent', () => {
 
   describe('createOpenTagMarkup', () => {
     function quoteRegexp(str) {
-      return (str + '').replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
+      return String(str).replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
     }
 
     function expectToHaveAttribute(actual, expected) {
@@ -1164,7 +1186,7 @@ describe('ReactDOMComponent', () => {
 
   describe('createContentMarkup', () => {
     function quoteRegexp(str) {
-      return (str + '').replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
+      return String(str).replace(/([.?*+\^$\[\]\\(){}|-])/g, '\\$1');
     }
 
     function genMarkup(props) {
@@ -2412,6 +2434,28 @@ describe('ReactDOMComponent', () => {
       expect(el.getAttribute('whatever')).toBe('[object Object]');
     });
 
+    it('allows Temporal-like objects as HTML (they are not coerced to strings first)', function() {
+      class TemporalLike {
+        valueOf() {
+          // Throwing here is the behavior of ECMAScript "Temporal" date/time API.
+          // See https://tc39.es/proposal-temporal/docs/plaindate.html#valueOf
+          throw new TypeError('prod message');
+        }
+        toString() {
+          return '2020-01-01';
+        }
+      }
+
+      // `dangerouslySetInnerHTML` is never coerced to a string, so won't throw
+      // even with a Temporal-like object.
+      const container = document.createElement('div');
+      ReactDOM.render(
+        <div dangerouslySetInnerHTML={{__html: new TemporalLike()}} />,
+        container,
+      );
+      expect(container.firstChild.innerHTML).toEqual('2020-01-01');
+    });
+
     it('allows cased data attributes', function() {
       let el;
       expect(() => {