[DevTools] Print component stacks as error objects to get source mapping #30289

sebmarkbage · 2024-07-05T22:21:08Z

Before:

After:

Firefox:

The first log doesn't get a stack because it's logged before DevTools boots up and connects which is unfortunate.

The second log already has a stack printed by React (this is on stable) it gets replaced by our object now.

The third and following logs don't have a stack and get one appended.

I only turn the stack into an error object if it matches what we would emit from DevTools anyway. Otherwise we assume it's not React. Since I had to change the format slightly to make this work, I first normalize the stack slightly before doing a comparison since it won't be 1:1.

vercel · 2024-07-05T22:21:12Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 8, 2024 10:34pm

sebmarkbage · 2024-07-05T22:21:58Z

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

@@ -40,6 +42,8 @@ export function describeFiber(
  } = workTagMap;

  switch (workInProgress.tag) {
+    case HostHoistable:
+    case HostSingleton:


This was missing previously leading to missing frames for html/body which lead to mismatches.

sebmarkbage · 2024-07-05T22:23:14Z

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

 }

 export function describeDebugInfoFrame(name: string, env: ?string): string {
-  return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : ''));
+  return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : ''));


Because parenthesis are parsed as part of the location in Chrome we need to replace those with brackets. Not sure why I used parenthesis in the first place really.

sebmarkbage · 2024-07-05T22:25:13Z

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

+    suffix = ' (<anonymous>)';
+  } else if (__IS_FIREFOX__) {
+    suffix = '@unknown:0:0';
+  }


Chrome parses the name as the location if it doesn't have one. <anonymous> is the only special case that works I think.

Firefox skips lines that don't have location so we need to add a fake one which can be anything really. I don't think there's any special case that doesn't linkify it.

Safari is fine without it.

@henrik

@henrik facebook#28659 @wvl facebook#28776 @qerub facebook#29665 @nickmerwin facebook#30289 @sverrejoh facebook#30259

hoxyq

Awesome. Left 1 clarifying question regarding filtering out %s from the end of the first arg.

hoxyq · 2024-07-08T12:27:47Z

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

+                          const firstArg = args[0];
+                          if (
+                            args.length > 1 &&
+                            typeof firstArg === 'string' &&
+                            firstArg.endsWith('%s')
+                          ) {
+                            args[0] = firstArg.slice(0, firstArg.length - 2); // Strip the %s param
+                          }


Is this the logic for backwards-compatibility with previous versions of React? To filter out %s at the end of the message.

If so, why is it gated with !isStrictModeOverride(...)? Shouldn't it be independent of when error was emitted?

If we're in the strict mode override we end up converting all parameterized arguments into a flat string. I'm not sure we really need to do that. E.g. for badging I tried to avoid doing that. One benefit of doing this is that we are guaranteed to be able to format the styling of all the content where as if you print them as object it might not listen to our color override.

So the formatting no longer has a trailing %s after it has been rewritten.

I think my intension was to just leave it as a string so that the color formatting consistently can override it. However, I was going to try it with %o. I think I messed up refactoring though so this doesn't quite make sense atm because it has already been flattened.

It's tricky to test because React should always dedupe so it doesn't get logged again during replaying.

If we're in the strict mode override we end up converting all parameterized arguments into a flat string.

Oh, true. Well, we can't really guarantee that it is going to be a single flat string, because we omit other objects and user-space console stylings and just bail out.

I'll just leave it intact in that case since it's non-essential and sometimes hidden anyway. It'll also go away with owner stacks since we'll never append then.

hoxyq · 2024-07-08T12:31:23Z

packages/react-devtools-inline/webpack.config.js

+      // TODO: Should this be feature tested somehow?
+      __IS_CHROME__: false,
+      __IS_FIREFOX__: false,
+      __IS_EDGE__: false,


Ugh, react-devtools-inline is shipped as an npm package, so these variables don't make much sense for it. Instead, it should be checked at runtime, not build time.

Will update this later

I'm not quite sure why more of these weren't reached and subsequently errored before. Not sure why just the ones I added mattered.

hoxyq · 2024-07-08T12:32:30Z

packages/react-devtools-core/webpack.backend.js

      __IS_FIREFOX__: false,
+      __IS_CHROME__: false,
+      __IS_EDGE__: false,


Will probably add __IS_NATIVE__ here as well.

This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks. Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example. While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object.

This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks. Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example. While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object. DiffTrain build for [df783f9](df783f9)

This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks. Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example. While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object. DiffTrain build for commit df783f9.

…t mode override We've already merged the stack with the message at this point so would need further refactoring to handle. This is non-essential and often just fully hidden.

sebmarkbage requested a review from hoxyq July 5, 2024 22:21

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jul 5, 2024

sebmarkbage commented Jul 5, 2024

View reviewed changes

vercel bot deployed to Preview July 5, 2024 22:23 View deployment

sebmarkbage commented Jul 5, 2024

View reviewed changes

sebmarkbage force-pushed the devtoolserrorstacks branch 2 times, most recently from 2b01aac to b12be26 Compare July 5, 2024 22:43

vercel bot deployed to Preview July 5, 2024 22:47 View deployment

sebmarkbage force-pushed the devtoolserrorstacks branch from b12be26 to 5297db2 Compare July 5, 2024 23:09

vercel bot deployed to Preview July 5, 2024 23:11 View deployment

sebmarkbage force-pushed the devtoolserrorstacks branch from 5297db2 to 31c9db8 Compare July 5, 2024 23:41

vercel bot deployed to Preview July 5, 2024 23:43 View deployment

GreggJEduardoPH093 added a commit to GreggJEduardoPH093/react that referenced this pull request Jul 7, 2024

Update SECURITY.md

e274be8

@henrik facebook#28659 @wvl facebook#28776 @qerub facebook#29665 @nickmerwin facebook#30289 @sverrejoh facebook#30259

sebmarkbage mentioned this pull request Jul 7, 2024

Add unknown location information to component stacks #30290

Merged

hoxyq approved these changes Jul 8, 2024

View reviewed changes

sebmarkbage added 2 commits July 8, 2024 16:49

Print component stacks as error objects to get source mapping

5c7871b

Don't try to format if the log already had a component stack in stric…

5b3ebf0

…t mode override We've already merged the stack with the message at this point so would need further refactoring to handle. This is non-essential and often just fully hidden.

sebmarkbage force-pushed the devtoolserrorstacks branch from 31c9db8 to 5b3ebf0 Compare July 8, 2024 22:32

vercel bot deployed to Preview July 8, 2024 22:34 View deployment

sebmarkbage merged commit 491a4ea into facebook:main Jul 8, 2024
139 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DevTools] Print component stacks as error objects to get source mapping #30289

[DevTools] Print component stacks as error objects to get source mapping #30289

sebmarkbage commented Jul 5, 2024

vercel bot commented Jul 5, 2024 •

edited

Loading

sebmarkbage Jul 5, 2024

sebmarkbage Jul 5, 2024

sebmarkbage Jul 5, 2024

hoxyq left a comment

hoxyq Jul 8, 2024

sebmarkbage Jul 8, 2024 •

edited

Loading

hoxyq Jul 8, 2024

sebmarkbage Jul 8, 2024

hoxyq Jul 8, 2024

sebmarkbage Jul 8, 2024

hoxyq Jul 8, 2024

[DevTools] Print component stacks as error objects to get source mapping #30289

[DevTools] Print component stacks as error objects to get source mapping #30289

Conversation

sebmarkbage commented Jul 5, 2024

vercel bot commented Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoxyq left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebmarkbage Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Jul 5, 2024 •

edited

Loading

sebmarkbage Jul 8, 2024 •

edited

Loading