[DevTools] Get source location from structured callsites in prepareStackTrace #33143
Conversation
github-actions
bot
added
the
React Core Team
| const line = | ||
| // $FlowFixMe[prop-missing] | ||
| typeof callSite.getEnclosingLineNumber === 'function' | ||
| ? (callSite: any).getEnclosingLineNumber() |
There was a problem hiding this comment.
This is the key. Now we're jumping to the beginning of the function.
|
One limitation is that once something else consumes the |
There was a problem hiding this comment.
One limitation is that once something else consumes the .stack such as if we print some error log that contains that component, then we've already lost the structured information so we no longer have the enclosing function.
Since we generate these Error objects, and React DevTools knows when it appends the owner stack, could we record the structured stacks somewhere and then maybe fetch it later, based on some DevToolsInstance id?
Edit: oh, this is applicable to all errors, not just synthetic ones that are appended by React DevTools. We can't patch throw, but since we patch console.error, we could handle these, but seems like an overkill for precise function header location.
| } | ||
|
|
||
| export function parseSourceFromOwnerStack(error: Error): Source | null { | ||
| // First attempt to collected the structured data using prepareStackTrace. |
There was a problem hiding this comment.
| // First attempt to collected the structured data using prepareStackTrace. | |
| // First attempt to collect the structured data using prepareStackTrace. |
| function collectStackTrace( | ||
| error: Error, | ||
| structuredStackTrace: CallSite[], | ||
| ): string { |
There was a problem hiding this comment.
Maybe worth leaving a link to V8 docs for this - https://v8.dev/docs/stack-trace-api#customizing-stack-traces.
| return (instance.source = parseSourceFromOwnerStack( | ||
| (unresolvedSource: any), | ||
| )); |
There was a problem hiding this comment.
If you would make isError a type guard, will it be enough to remove the any type casting?
function isError(object: mixed): object is Error {
return toString.call(object) === '[object Error]';
}
| // At the same time we generate a string stack trace just in case someone | ||
| // else reads it. | ||
| const name = error.name || 'Error'; | ||
| const message = error.message || ''; | ||
| let stack = name + ': ' + message; | ||
| for (let i = 0; i < structuredStackTrace.length; i++) { | ||
| stack += '\n at ' + structuredStackTrace[i].toString(); | ||
| } | ||
| return stack; |
There was a problem hiding this comment.
I don't get how this can be read? You patched Error.prepareStackTrace with this function, so React DevTools is essentially the only user of this while it is installed, because you are not bubbling it up to previous function declared in Error.prepareStackTrace.
…ackTrace (#33143) When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component. This is similar to the technique in #33136. This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered). We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better. DiffTrain build for [997c7bc](997c7bc)
…ackTrace (facebook#33143) When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component. This is similar to the technique in facebook#33136. This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered). We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call `inspect(fn)` at least in Chrome which is even better. DiffTrain build for [997c7bc](facebook@997c7bc)
When we get the source location for "View source for this element" we should be using the enclosing function of the callsite of the child. So that we don't just point to some random line within the component.
This is similar to the technique in #33136.
This technique is now really better than the fake throw technique, when available. So I now favor the owner technique. The only problem it's only available in DEV and only if it has a child that's owned (and not filtered).
We could implement this same technique for the error that's thrown in the fake throwing solution. However, we really shouldn't need that at all because for client components we should be able to call
inspect(fn)at least in Chrome which is even better.