facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.58k stars 46.43k forks source link

DevTools: Provide full file path for React Native component stacks #17553

Open rickhanlonii opened 4 years ago

rickhanlonii commented 4 years ago

Overview

In React Native, we're working on a new RedBox experience for errors and warnings called LogBox. In LogBox, we separate out component stack traces and show them similar to call stacks.

We'd like to be able to tap on these components and open them (like we can with call stacks).

Solutions

In React we have the full file path context, but when we build the component stack trace, we strip the full path so that it displays only the file name.

There are two options to achieve this:

bvaughn commented 4 years ago

Any changes to the DevTools fork of describeComponentFrame would not impact built-in React warnings from shared/describeComponentFrame.

Seems like that might be a show-stopper. We could change both (maybe with a renderer-specific build flag?) but that would have a broader impact and we might want to give more consideration.

Keep the message the same, and instead add structured component stack frame info including the full file path.

What did you have in mind for this option?

rickhanlonii commented 4 years ago

It's possible that I linked to the wrong describeComponentFrame, could we update one of them so that React Native gets the full path?

cc @motiz88 ideas around the structured stacks

motiz88 commented 4 years ago

The concrete solution for structured stack traces still needs to be designed, but in my view it boils down to this: RN's LogBox and ReactFiberErrorDialog both need to have access to a Fiber reference (or maybe an array of ReactElements or some other intermediate projection) instead of a preformatted string for the component stack.

For the ReactFiberErrorDialog path, this amounts to refactoring internal interfaces within React and the renderers. For the LogBox path, we may need to rethink how we wrap and forward console.error and console.warn. For instance, maybe we can wrap the methods just once, in LogBox, and directly consume the same private API that React DevTools uses to get at the current fiber.

bvaughn commented 4 years ago

It's possible that I linked to the wrong describeComponentFrame, could we update one of them so that React Native gets the full path?

To clarify, the limitation I mentioned above was that warnings can be logged from two places:

I could add a flag that changes the behavior in DevTools (which would affect the stacks that get auto-appended to custom / third-party warnings) but the behavior in React core (for warnings like e.g. missing key prop) would not be affected. Since that behavior is in the react core package, I'm not sure of a great way to address it on a per-renderer level.

We could potentially add an expando prop somewhere to pass the non-trimmed paths through, but that feels pretty hacky- and based on the above comment from @motiz88, it seems like maybe the three of us aren't quite aligned about what the ideal solution is here. Maybe we should all three chat sometime (maybe loop in Sebastian too if he's interested).

For the LogBox path, we may need to rethink how we wrap and forward console.error and console.warn. For instance, maybe we can wrap the methods just once, in LogBox, and directly consume the same private API that React DevTools uses to get at the current fiber.

This has implications for third party warnings (something DevTools currently "supports" by auto-appending component stacks). Ideally, whichever solution we go with should support both the built-in warnings and third-party warnings.