Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Improve console dimming on second StrictMode render #24306

Closed
wants to merge 1 commit into from

Conversation

billyjanitsch
Copy link
Contributor

Summary

Fixes #24302. The goal is to apply dimming only to string values to avoid interfering with printing and introspection of complex values.

This PR isn't ready for review yet. Putting it up to show a working implementation, but:

  1. It can probably be simplified.
  2. It needs tests.
  3. I need to figure out what to do with the synchronized format implementation in the backend/ directory. The one in hook.js has only one callsite but the one in backend/ has several, some of which seem unrelated to browser console formatting. I don't have any knowledge of the devtools architecture so it'll take me some time to understand.
  4. I'm a bit concerned that this implementation only works when it agrees with the runtime about the list of placeholder symbols. This is also true of the existing implementation, but it would be nice to find a way around that, or at least to double check that this list is consistent across browsers + Node.

Implementation notes

The console API has two forms:

  1. Placeholder form, where the first N+1 arguments are a string containing N placeholders and N placeholder values, followed by any number of arbitrary values, e.g., ['%cA %s', 'color: blue', 'B', 'C'].
  2. Non-placeholder form, where all arguments are arbitrary values, e.g., ['A', 'B', 'C'].

Colors can only be applied using the first form, and only to the contents of the placeholder string. So the basic idea is to coerce the argument list into the placeholder form, apply the color directives, and append additional placeholders for all arguments without one. Some examples of this translation:

  • ['A', 1, 'B'] => ['%c%s %o %s', 'color: X', 'A', 1, 'B']
  • ['A %d', 1, 'B'] => ['%cA %d %c%s', 'color: X', 1, 'color: X', 'B']
  • ['A%cB', 'color: blue', 'C'] => ['%cA%cB %c%s', 'color: X', 'color: blue', 'color: X', 'C']

How did you test this change?

TBD

@billyjanitsch
Copy link
Contributor Author

Superseded by #24373.

@billyjanitsch billyjanitsch deleted the console-dimming branch April 14, 2022 00:23
lunaruan added a commit that referenced this pull request Apr 14, 2022
Fixes #24302 based on #24306.
---

The current implementation for strict mode double logging stringiness and dims the second log. However, because we stringify everything, including objects, this causes objects to be logged as `[object Object]` etc.

This PR creates a new function that formats console log arguments with a specified style. It does this by:
1. The first param is a string that contains %c: Bail out and return the args without modifying the styles. We don't want to affect styles that the developer deliberately set.
2. The first param is a string that doesn't contain %c but contains string formatting: `[`%c${args[0]}`, style, ...args.slice(1)]` Note: we assume that the string formatting that the developer uses is correct.
3. The first param is a string that doesn't contain string formatting OR is not a string: Create a formatting string where:
   -  boolean, string, symbol -> %s
   -  number -> %f OR %i depending on if it's an int or float
   -  default -> %o
---
Co-authored-by: Billy Janitsch <billy@kensho.com>
lunaruan added a commit to lunaruan/react that referenced this pull request Apr 14, 2022
Fixes facebook#24302 based on facebook#24306.
---

The current implementation for strict mode double logging stringiness and dims the second log. However, because we stringify everything, including objects, this causes objects to be logged as `[object Object]` etc.

This PR creates a new function that formats console log arguments with a specified style. It does this by:
1. The first param is a string that contains %c: Bail out and return the args without modifying the styles. We don't want to affect styles that the developer deliberately set.
2. The first param is a string that doesn't contain %c but contains string formatting: `[`%c${args[0]}`, style, ...args.slice(1)]` Note: we assume that the string formatting that the developer uses is correct.
3. The first param is a string that doesn't contain string formatting OR is not a string: Create a formatting string where:
   -  boolean, string, symbol -> %s
   -  number -> %f OR %i depending on if it's an int or float
   -  default -> %o
---

Co-authored-by: Billy Janitsch <billy@kensho.com>
rickhanlonii pushed a commit that referenced this pull request Apr 14, 2022
Fixes #24302 based on #24306.
---

The current implementation for strict mode double logging stringiness and dims the second log. However, because we stringify everything, including objects, this causes objects to be logged as `[object Object]` etc.

This PR creates a new function that formats console log arguments with a specified style. It does this by:
1. The first param is a string that contains %c: Bail out and return the args without modifying the styles. We don't want to affect styles that the developer deliberately set.
2. The first param is a string that doesn't contain %c but contains string formatting: `[`%c${args[0]}`, style, ...args.slice(1)]` Note: we assume that the string formatting that the developer uses is correct.
3. The first param is a string that doesn't contain string formatting OR is not a string: Create a formatting string where:
   -  boolean, string, symbol -> %s
   -  number -> %f OR %i depending on if it's an int or float
   -  default -> %o
---
Co-authored-by: Billy Janitsch <billy@kensho.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console dimming on second StrictMode render forces string cast
2 participants