[Flight] Prefix Replayed Console Logs with a Badge #28403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
force-pushed
the
flightlogstyling
branch
from
836ed59
to
dc27b87
Compare
|
The badges are sick. 🔥 |
hoxyq
reviewed
Feb 21, 2024
Comment on lines
+30
to
+42
| case 'dir': | ||
| case 'dirxml': | ||
| case 'groupEnd': | ||
| case 'table': { | ||
| // These methods cannot be colorized because they don't take a formatting string. | ||
| // eslint-disable-next-line react-internal/no-production-logging | ||
| console[methodName].apply(console, args); | ||
| return; | ||
| } | ||
| case 'assert': { | ||
| // assert takes formatting options as the second argument. | ||
| offset = 1; | ||
| } |
There was a problem hiding this comment.
Do we allowlist these methods somewhere? I wonder what the behaviour will be if some non-standard console methods are called, like console.timeStamp or console.createTask?
There was a problem hiding this comment.
I think I found the answer - https://github.com/facebook/react/pull/28384/files#r1495197550
There was a problem hiding this comment.
Yes, we only patch a subset. The list and what was excluded is commented on here:
hoxyq
approved these changes
Feb 21, 2024
Chrome + Terminals tend to support color encoding in ANSI SGR format. Safari + Chrome + Firefox supports CSS styling encoding. Therefore in browser settings we need to use CSS styling but on servers we have to use ANSI SGR. You can ofc use a browser DevTools inspector to inspect a server, however, in that case it's probably Chrome which supports ANSI SGR too.
On the server we can use a combination of ANSI and CSS. The ANSI is used in the terminal while the CSS is ignored, the CSS overrides the ANSI in Chrome inspector. For ANSI we use the inverted colors (flip foreground/background) instead of any specific set of colors since we don't know what color scheme is used. In the browser, we can't use ANSI because Safari and Firefox will print them in the inspector, but we can use CSS there.
This works a bit better for multi-line logs. This means we also don't need normalizeConsoleFormat but I keep it around in case we change our mind or need it for other logs.
In the terminal it prints the CSS and in the inspector it prints the ANSI. So we just use a plain bracket instead.
sebmarkbage
force-pushed
the
flightlogstyling
branch
from
dc27b87
to
525b3e5
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Feb 21, 2024
Builds on top of #28384. This prefixes each log with a badge similar to how we badge built-ins like "ForwardRef" and "Memo" in the React DevTools. The idea is that we can add such badges in DevTools for Server Components too to carry on the consistency. This puts the "environment" name in the badge which defaults to "Server". So you know which source it is coming from. We try to use the same styling as the React DevTools. We use light-dark mode where available to support the two different color styles, but if it's not available I use a fixed background so that it's always readable even in dark mode. In Terminals, instead of hard coding colors that might not look good with some themes, I use the ANSI color code to flip background/foreground colors in that case. In earlier commits I had it on the end of the line similar to the DevTools badges but for multiline I found it better to prefix it. We could try various options tough. In most cases we can use both ANSI and the `%c` CSS color specifier, because node will only use ANSI and hide the other. Chrome supports both but the color overrides ANSI if it comes later (and Chrome doesn't support color inverting anyway). Safari/Firefox prints the ANSI, so it can only use CSS colors. Therefore in browser builds I exclude ANSI. On the server I support both so if you use Chrome inspector on the server, you get nice colors on both terminal and in the inspector. Since Bun uses WebKit inspector and it prints the ANSI we can't safely emit both there. However, we also can't emit just the color specifier because then it prints in the terminal. oven-sh/bun#9021 So we just use a plain string prefix for now with a bracket until that's fixed. Screen shots: <img width="758" alt="Screenshot 2024-02-21 at 12 56 02 AM" src="https://github.com/facebook/react/assets/63648/4f887ffe-fffe-4402-bf2a-b7890986d60c"> <img width="759" alt="Screenshot 2024-02-21 at 12 56 24 AM" src="https://github.com/facebook/react/assets/63648/f32d432f-f738-4872-a700-ea0a78e6c745"> <img width="514" alt="Screenshot 2024-02-21 at 12 57 10 AM" src="https://github.com/facebook/react/assets/63648/205d2e82-75b7-4e2b-9d9c-aa9e2cbedf39"> <img width="489" alt="Screenshot 2024-02-21 at 12 57 34 AM" src="https://github.com/facebook/react/assets/63648/ea52d1e4-b9fa-431d-ae9e-ccb87631f399"> <img width="516" alt="Screenshot 2024-02-21 at 12 58 23 AM" src="https://github.com/facebook/react/assets/63648/52b50fac-bec0-471d-a457-1a10d8df9172"> <img width="956" alt="Screenshot 2024-02-21 at 12 58 56 AM" src="https://github.com/facebook/react/assets/63648/0096ed61-5eff-4aa9-8a8a-2204e754bd1f"> DiffTrain build for [c027406](c027406)
|
This is sick |
huozhi
added a commit
to vercel/next.js
that referenced
this pull request
Feb 26, 2024
### React upstream changes - facebook/react#28438 - facebook/react#28436 - facebook/react#25954 - facebook/react#28434 - facebook/react#28433 - facebook/react#28432 - facebook/react#28415 - facebook/react#27903 - facebook/react#28430 - facebook/react#28424 - facebook/react#28400 - facebook/react#28422 - facebook/react#28423 - facebook/react#28412 - facebook/react#28418 - facebook/react#28421 - facebook/react#28417 - facebook/react#28399 - facebook/react#28408 - facebook/react#28350 - facebook/react#28387 - facebook/react#28403 - facebook/react#28384 - facebook/react#28409 - facebook/react#28398 - facebook/react#28405 - facebook/react#28328 - facebook/react#28402 - facebook/react#28386 - facebook/react#28388 - facebook/react#28379 - facebook/react#28383 - facebook/react#28390 - facebook/react#28389 - facebook/react#28382 - facebook/react#28348 Closes NEXT-2600
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
Builds on top of facebook#28384. This prefixes each log with a badge similar to how we badge built-ins like "ForwardRef" and "Memo" in the React DevTools. The idea is that we can add such badges in DevTools for Server Components too to carry on the consistency. This puts the "environment" name in the badge which defaults to "Server". So you know which source it is coming from. We try to use the same styling as the React DevTools. We use light-dark mode where available to support the two different color styles, but if it's not available I use a fixed background so that it's always readable even in dark mode. In Terminals, instead of hard coding colors that might not look good with some themes, I use the ANSI color code to flip background/foreground colors in that case. In earlier commits I had it on the end of the line similar to the DevTools badges but for multiline I found it better to prefix it. We could try various options tough. In most cases we can use both ANSI and the `%c` CSS color specifier, because node will only use ANSI and hide the other. Chrome supports both but the color overrides ANSI if it comes later (and Chrome doesn't support color inverting anyway). Safari/Firefox prints the ANSI, so it can only use CSS colors. Therefore in browser builds I exclude ANSI. On the server I support both so if you use Chrome inspector on the server, you get nice colors on both terminal and in the inspector. Since Bun uses WebKit inspector and it prints the ANSI we can't safely emit both there. However, we also can't emit just the color specifier because then it prints in the terminal. oven-sh/bun#9021 So we just use a plain string prefix for now with a bracket until that's fixed. Screen shots: <img width="758" alt="Screenshot 2024-02-21 at 12 56 02 AM" src="https://github.com/facebook/react/assets/63648/4f887ffe-fffe-4402-bf2a-b7890986d60c"> <img width="759" alt="Screenshot 2024-02-21 at 12 56 24 AM" src="https://github.com/facebook/react/assets/63648/f32d432f-f738-4872-a700-ea0a78e6c745"> <img width="514" alt="Screenshot 2024-02-21 at 12 57 10 AM" src="https://github.com/facebook/react/assets/63648/205d2e82-75b7-4e2b-9d9c-aa9e2cbedf39"> <img width="489" alt="Screenshot 2024-02-21 at 12 57 34 AM" src="https://github.com/facebook/react/assets/63648/ea52d1e4-b9fa-431d-ae9e-ccb87631f399"> <img width="516" alt="Screenshot 2024-02-21 at 12 58 23 AM" src="https://github.com/facebook/react/assets/63648/52b50fac-bec0-471d-a457-1a10d8df9172"> <img width="956" alt="Screenshot 2024-02-21 at 12 58 56 AM" src="https://github.com/facebook/react/assets/63648/0096ed61-5eff-4aa9-8a8a-2204e754bd1f">
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
Builds on top of #28384. This prefixes each log with a badge similar to how we badge built-ins like "ForwardRef" and "Memo" in the React DevTools. The idea is that we can add such badges in DevTools for Server Components too to carry on the consistency. This puts the "environment" name in the badge which defaults to "Server". So you know which source it is coming from. We try to use the same styling as the React DevTools. We use light-dark mode where available to support the two different color styles, but if it's not available I use a fixed background so that it's always readable even in dark mode. In Terminals, instead of hard coding colors that might not look good with some themes, I use the ANSI color code to flip background/foreground colors in that case. In earlier commits I had it on the end of the line similar to the DevTools badges but for multiline I found it better to prefix it. We could try various options tough. In most cases we can use both ANSI and the `%c` CSS color specifier, because node will only use ANSI and hide the other. Chrome supports both but the color overrides ANSI if it comes later (and Chrome doesn't support color inverting anyway). Safari/Firefox prints the ANSI, so it can only use CSS colors. Therefore in browser builds I exclude ANSI. On the server I support both so if you use Chrome inspector on the server, you get nice colors on both terminal and in the inspector. Since Bun uses WebKit inspector and it prints the ANSI we can't safely emit both there. However, we also can't emit just the color specifier because then it prints in the terminal. oven-sh/bun#9021 So we just use a plain string prefix for now with a bracket until that's fixed. Screen shots: <img width="758" alt="Screenshot 2024-02-21 at 12 56 02 AM" src="https://github.com/facebook/react/assets/63648/4f887ffe-fffe-4402-bf2a-b7890986d60c"> <img width="759" alt="Screenshot 2024-02-21 at 12 56 24 AM" src="https://github.com/facebook/react/assets/63648/f32d432f-f738-4872-a700-ea0a78e6c745"> <img width="514" alt="Screenshot 2024-02-21 at 12 57 10 AM" src="https://github.com/facebook/react/assets/63648/205d2e82-75b7-4e2b-9d9c-aa9e2cbedf39"> <img width="489" alt="Screenshot 2024-02-21 at 12 57 34 AM" src="https://github.com/facebook/react/assets/63648/ea52d1e4-b9fa-431d-ae9e-ccb87631f399"> <img width="516" alt="Screenshot 2024-02-21 at 12 58 23 AM" src="https://github.com/facebook/react/assets/63648/52b50fac-bec0-471d-a457-1a10d8df9172"> <img width="956" alt="Screenshot 2024-02-21 at 12 58 56 AM" src="https://github.com/facebook/react/assets/63648/0096ed61-5eff-4aa9-8a8a-2204e754bd1f"> DiffTrain build for commit c027406.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on top of #28384.
This prefixes each log with a badge similar to how we badge built-ins like "ForwardRef" and "Memo" in the React DevTools. The idea is that we can add such badges in DevTools for Server Components too to carry on the consistency.
This puts the "environment" name in the badge which defaults to "Server". So you know which source it is coming from.
We try to use the same styling as the React DevTools. We use light-dark mode where available to support the two different color styles, but if it's not available I use a fixed background so that it's always readable even in dark mode.
In Terminals, instead of hard coding colors that might not look good with some themes, I use the ANSI color code to flip background/foreground colors in that case.
In earlier commits I had it on the end of the line similar to the DevTools badges but for multiline I found it better to prefix it. We could try various options tough.
In most cases we can use both ANSI and the
%cCSS color specifier, because node will only use ANSI and hide the other. Chrome supports both but the color overrides ANSI if it comes later (and Chrome doesn't support color inverting anyway). Safari/Firefox prints the ANSI, so it can only use CSS colors.Therefore in browser builds I exclude ANSI.
On the server I support both so if you use Chrome inspector on the server, you get nice colors on both terminal and in the inspector.
Since Bun uses WebKit inspector and it prints the ANSI we can't safely emit both there. However, we also can't emit just the color specifier because then it prints in the terminal. oven-sh/bun#9021 So we just use a plain string prefix for now with a bracket until that's fixed.
Screen shots: