chore[react-devtools]: improve console arguments formatting before pa…

…ssing it to original console

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

@@ -14,7 +14,8 @@ import {
 } from 'react-devtools-shared/src/utils';
 import {stackToComponentSources} from 'react-devtools-shared/src/devtools/utils';
 import {
-  format,
+  formatConsoleArguments,
+  formatConsoleArgumentsToSingleString,
   formatWithStyles,
   gt,
   gte,
@@ -123,51 +124,51 @@ describe('utils', () => {
     });
   });
 
-  describe('format', () => {
-    // @reactVersion >= 16.0
+  describe('formatConsoleArgumentsToSingleString', () => {
     it('should format simple strings', () => {
-      expect(format('a', 'b', 'c')).toEqual('a b c');
+      expect(formatConsoleArgumentsToSingleString('a', 'b', 'c')).toEqual(
+        'a b c',
+      );
     });
 
-    // @reactVersion >= 16.0
     it('should format multiple argument types', () => {
-      expect(format('abc', 123, true)).toEqual('abc 123 true');
+      expect(formatConsoleArgumentsToSingleString('abc', 123, true)).toEqual(
+        'abc 123 true',
+      );
     });
 
-    // @reactVersion >= 16.0
     it('should support string substitutions', () => {
-      expect(format('a %s b %s c', 123, true)).toEqual('a 123 b true c');
+      expect(
+        formatConsoleArgumentsToSingleString('a %s b %s c', 123, true),
+      ).toEqual('a 123 b true c');
     });
 
-    // @reactVersion >= 16.0
     it('should gracefully handle Symbol types', () => {
-      expect(format(Symbol('a'), 'b', Symbol('c'))).toEqual(
-        'Symbol(a) b Symbol(c)',
-      );
+      expect(
+        formatConsoleArgumentsToSingleString(Symbol('a'), 'b', Symbol('c')),
+      ).toEqual('Symbol(a) b Symbol(c)');
     });
 
-    // @reactVersion >= 16.0
     it('should gracefully handle Symbol type for the first argument', () => {
-      expect(format(Symbol('abc'), 123)).toEqual('Symbol(abc) 123');
+      expect(formatConsoleArgumentsToSingleString(Symbol('abc'), 123)).toEqual(
+        'Symbol(abc) 123',
+      );
     });
   });
 
   describe('formatWithStyles', () => {
-    // @reactVersion >= 16.0
     it('should format empty arrays', () => {
       expect(formatWithStyles([])).toEqual([]);
       expect(formatWithStyles([], 'gray')).toEqual([]);
       expect(formatWithStyles(undefined)).toEqual(undefined);
     });
 
-    // @reactVersion >= 16.0
     it('should bail out of strings with styles', () => {
       expect(
         formatWithStyles(['%ca', 'color: green', 'b', 'c'], 'color: gray'),
       ).toEqual(['%ca', 'color: green', 'b', 'c']);
     });
 
-    // @reactVersion >= 16.0
     it('should format simple strings', () => {
       expect(formatWithStyles(['a'])).toEqual(['a']);
 
@@ -186,7 +187,6 @@ describe('utils', () => {
       ]);
     });
 
-    // @reactVersion >= 16.0
     it('should format string substituions', () => {
       expect(
         formatWithStyles(['%s %s %s', 'a', 'b', 'c'], 'color: gray'),
@@ -199,7 +199,6 @@ describe('utils', () => {
       );
     });
 
-    // @reactVersion >= 16.0
     it('should support multiple argument types', () => {
       const symbol = Symbol('a');
       expect(
@@ -219,7 +218,6 @@ describe('utils', () => {
       ]);
     });
 
-    // @reactVersion >= 16.0
     it('should properly format escaped string substituions', () => {
       expect(formatWithStyles(['%%s'], 'color: gray')).toEqual([
         '%c%s',
@@ -234,7 +232,6 @@ describe('utils', () => {
       expect(formatWithStyles(['%%c%c'], 'color: gray')).toEqual(['%%c%c']);
     });
 
-    // @reactVersion >= 16.0
     it('should format non string inputs as the first argument', () => {
       expect(formatWithStyles([{foo: 'bar'}])).toEqual([{foo: 'bar'}]);
       expect(formatWithStyles([[1, 2, 3]])).toEqual([[1, 2, 3]]);
@@ -387,4 +384,55 @@ describe('utils', () => {
       });
     });
   });
+
+  describe('formatConsoleArguments', () => {
+    it('works with empty arguments list', () => {
+      expect(formatConsoleArguments(...[])).toEqual([]);
+    });
+
+    it('works for string without escape sequences', () => {
+      expect(
+        formatConsoleArguments('This is the template', 'And another string'),
+      ).toEqual(['This is the template', 'And another string']);
+    });
+
+    it('works with strings templates', () => {
+      expect(formatConsoleArguments('This is %s template', 'the')).toEqual([
+        'This is the template',
+      ]);
+    });
+
+    it('skips %%s', () => {
+      expect(formatConsoleArguments('This %%s is %s template', 'the')).toEqual([
+        'This %%s is the template',
+      ]);
+    });
+
+    it('works with %%%s', () => {
+      expect(
+        formatConsoleArguments('This %%%s is %s template', 'test', 'the'),
+      ).toEqual(['This %%test is the template']);
+    });
+
+    it("doesn't inline objects", () => {
+      expect(
+        formatConsoleArguments('This is %s template with object %o', 'the', {}),
+      ).toEqual(['This is the template with object %o', {}]);
+    });
+
+    it("doesn't inline css", () => {
+      expect(
+        formatConsoleArguments(
+          'This is template with %c %s object %o',
+          'color: rgba(...)',
+          'the',
+          {},
+        ),
+      ).toEqual([
+        'This is template with %c the object %o',
+        'color: rgba(...)',
+        {},
+      ]);
+    });
+  });
 });

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

@@ -15,8 +15,11 @@ import type {
   WorkTagMap,
   ConsolePatchSettings,
 } from './types';
-import {formatWithStyles} from './utils';
 
+import {
+  formatConsoleArguments,
+  formatWithStyles,
+} from 'react-devtools-shared/src/backend/utils';
 import {
   FIREFOX_CONSOLE_DIMMING_COLOR,
   ANSI_STYLE_DIMMING_TEMPLATE,
@@ -335,7 +338,10 @@ export function patchForStrictMode() {
               ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
             );
           } else {
-            originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
+            originalMethod(
+              ANSI_STYLE_DIMMING_TEMPLATE,
+              ...formatConsoleArguments(...args),
+            );
           }
         }
       };

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

@@ -40,6 +40,7 @@ import {
 } from 'react-devtools-shared/src/utils';
 import {sessionStorageGetItem} from 'react-devtools-shared/src/storage';
 import {
+  formatConsoleArgumentsToSingleString,
   gt,
   gte,
   parseSourceFromComponentStack,
@@ -95,7 +96,6 @@ import {
   MEMO_SYMBOL_STRING,
   SERVER_CONTEXT_SYMBOL_STRING,
 } from './ReactSymbols';
-import {format} from './utils';
 import {enableStyleXFeatures} from 'react-devtools-feature-flags';
 import is from 'shared/objectIs';
 import hasOwnProperty from 'shared/hasOwnProperty';
@@ -841,7 +841,14 @@ export function attach(
         return;
       }
     }
-    const message = format(...args);
+
+    // We can't really use this message as a unique key, since we can't distinguish
+    // different objects in this implementation. We have to delegate displaying of the objects
+    // to the environment, the browser console, for example, so this is why this should be kept
+    // as an array of arguments, instead of the plain string.
+    // [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message,
+    // even if objects are different
+    const message = formatConsoleArgumentsToSingleString(...args);
     if (__DEBUG__) {
       debug('onErrorOrWarning', fiber, null, `${type}: "${message}"`);
     }

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

@@ -164,6 +164,7 @@ export function serializeToString(data: any): string {
   );
 }
 
+// NOTE: KEEP IN SYNC with src/hook.js
 // Formats an array of args with a style for console methods, using
 // the following algorithm:
 //     1. The first param is a string that contains %c
@@ -220,11 +221,72 @@ export function formatWithStyles(
   }
 }
 
+// NOTE: KEEP IN SYNC with src/hook.js
+// Skips CSS and object arguments, inlines other in the first argument as a template string
+export function formatConsoleArguments(
+  maybeMessage: any,
+  ...inputArgs: $ReadOnlyArray<any>
+): $ReadOnlyArray<any> {
+  if (inputArgs.length === 0 || typeof maybeMessage !== 'string') {
+    return [maybeMessage, ...inputArgs];
+  }
+
+  const args = inputArgs.slice();
+
+  let template = '';
+  let argumentsPointer = 0;
+  for (let i = 0; i < maybeMessage.length; ++i) {
+    const currentChar = maybeMessage[i];
+    if (currentChar !== '%') {
+      template += currentChar;
+      continue;
+    }
+
+    const nextChar = maybeMessage[i + 1];
+    ++i;
+
+    // Only keep CSS and objects, inline other arguments
+    switch (nextChar) {
+      case 'c':
+      case 'O':
+      case 'o': {
+        ++argumentsPointer;
+        template += `%${nextChar}`;
+
+        break;
+      }
+      case 'd':
+      case 'i': {
+        const [arg] = args.splice(argumentsPointer, 1);
+        template += parseInt(arg, 10).toString();
+
+        break;
+      }
+      case 'f': {
+        const [arg] = args.splice(argumentsPointer, 1);
+        template += parseFloat(arg).toString();
+
+        break;
+      }
+      case 's': {
+        const [arg] = args.splice(argumentsPointer, 1);
+        template += arg.toString();
+
+        break;
+      }
+
+      default:
+        template += `%${nextChar}`;
+    }
+  }
+
+  return [template, ...args];
+}
+
 // based on https://github.com/tmpfs/format-util/blob/0e62d430efb0a1c51448709abd3e2406c14d8401/format.js#L1
 // based on https://developer.mozilla.org/en-US/docs/Web/API/console#Using_string_substitutions
 // Implements s, d, i and f placeholders
-// NOTE: KEEP IN SYNC with src/hook.js
-export function format(
+export function formatConsoleArgumentsToSingleString(
   maybeMessage: any,
   ...inputArgs: $ReadOnlyArray<any>
 ): string {

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

@@ -220,6 +220,61 @@ export function installHook(target: any): DevToolsHook | null {
       return [firstArg, style, ...inputArgs];
     }
   }
+  // NOTE: KEEP IN SYNC with src/backend/utils.js
+  function formatConsoleArguments(
+    maybeMessage: any,
+    ...inputArgs: $ReadOnlyArray<any>
+  ): $ReadOnlyArray<any> {
+    if (inputArgs.length === 0 || typeof maybeMessage !== 'string') {
+      return [maybeMessage, ...inputArgs];
+    }
+
+    const args = inputArgs.slice();
+
+    let template = '';
+    let argumentsPointer = 0;
+    for (let i = 0; i < maybeMessage.length; ++i) {
+      const currentChar = maybeMessage[i];
+      if (currentChar !== '%') {
+        template += currentChar;
+        continue;
+      }
+
+      const nextChar = maybeMessage[i + 1];
+      ++i;
+
+      // Only keep CSS and objects, inline other arguments
+      switch (nextChar) {
+        case 'c':
+        case 'O':
+        case 'o': {
+          ++argumentsPointer;
+          template += `%${nextChar}`;
+
+          break;
+        }
+        case 'd':
+        case 'i': {
+          const [arg] = args.splice(argumentsPointer, 1);
+          template += parseInt(arg, 10).toString();
+
+          break;
+        }
+        case 'f': {
+          const [arg] = args.splice(argumentsPointer, 1);
+          template += parseFloat(arg).toString();
+
+          break;
+        }
+        case 's': {
+          const [arg] = args.splice(argumentsPointer, 1);
+          template += arg.toString();
+        }
+      }
+    }
+
+    return [template, ...args];
+  }
 
   let unpatchFn = null;
 
@@ -274,7 +329,10 @@ export function installHook(target: any): DevToolsHook | null {
                 ...formatWithStyles(args, FIREFOX_CONSOLE_DIMMING_COLOR),
               );
             } else {
-              originalMethod(ANSI_STYLE_DIMMING_TEMPLATE, ...args);
+              originalMethod(
+                ANSI_STYLE_DIMMING_TEMPLATE,
+                ...formatConsoleArguments(...args),
+              );
             }
           }
         };