react-native
react-native copied to clipboard
fix component stack symbolication in terminal
Summary:
Use the parsed component stack logic that's used for the logbox UI to generate the component stack logs in the terminal. This increases the correctness of the message between the terminal and the UI.
Before
After
UI (unchanged)
Notice that html.h1
is the first frame in the component stack in the UI:
Further work could figure out why component frames inside of certain node modules, in this case react-native
Text, doesn't symbolicate correctly (at which point we can ignore the frame haha).
Changelog:
Test Plan:
- New project:
bunx create-expo-app -t blank@50
- Add dependency:
bun install react-strict-dom
-
bunx expo --ios
- Usage of invalid style should warn:
import { StatusBar } from "expo-status-bar";
import { StyleSheet, View } from "react-native";
import { html } from "react-strict-dom";
export default function App() {
return (
<View style={styles.container}>
<html.h1>Open up App.js to start working on your app!</html.h1>
<StatusBar style="auto" />
</View>
);
}
const styles = StyleSheet.create({
container: {
flex: 1,
backgroundColor: "#fff",
alignItems: "center",
justifyContent: "center",
},
});
- The terminal warning should now match the results in the LogBox UI.
ack, i'm actually working on this right now
Yeah I don't think this is the right fix, because this doesn't fix the non-symbolicated non-component stack traces, and LogBox skips certain logs (such as ignored logs) which would still display in the console. So I think Metro should symbolicate the logs, the way browser devtools handle symbolicating. But I'm curious what @motiz88 thinks.
this doesn't fix the non-symbolicated non-component stack traces
I agree that solving for both makes the most sense. Showing a partially symbolicated trace feels broken.
Metro should symbolicate the logs, the way browser devtools handle symbolicating
Speaking of browser devtools, we're considering phasing out the logs-in-Metro feature once we ship the new Chrome DevTools-based debugger as stable. (This is currently slated for after 0.75, i.e. later this year.) With that in mind I don't think it is super valuable to optimise the way things are displayed in the terminal, especially where it adds new complexity (like stack trace parsing in Metro, which is currently not a thing).
@rickhanlonii I added the "Further work could figure out why component frames inside of certain node modules, in this case react-native Text, doesn't symbolicate correctly (at which point we can ignore the frame haha).".
This proposed change introduces the use of parsed component stacks which can be extended in the future to support filtering, whereas the current behavior is to simply print the stack as provided by the JS engine. When the error is more robust, there tend to be a ton of these unsymbolicated logs, which hide the more useful info.
Speaking of browser devtools, we're considering phasing out the logs-in-Metro feature once we ship the new Chrome DevTools-based debugger as stable. (This is currently slated for after 0.75, i.e. later this year.)
Seems like we should wait for adoption of the new chrome devtools stuff to be reasonably adequate before stopping improvements on the existing system.
@EvanBacon the right place to do that would be in parseLogBoxLog here:
https://github.com/facebook/react-native/blob/main/packages/react-native/Libraries/LogBox/Data/parseLogBoxLog.js#L172
I think I know what's happening with text - React changed the way it does component stack frames, but React Native didn't have that turned on, so there's probably a weird mixture somewhere. I'm working on fixing that now in https://github.com/facebook/react-native/pull/43165 and https://github.com/facebook/react-native/pull/43166. This fix will call into Metro to symbolicate the component stack frames the same way we do call stack frames.
Can you write a test case or share a full stack trace so I can make sure it's handled?
Oh, I see the stack in the screenshot. Yeah, that first frame is using the new stack frame based component stacks. My PRs replace this.