[Flight] Encode enclosing line/column numbers and use it to align the fake function #33136
Conversation
| action = createFakeServerFunction( | ||
| functionName, | ||
| filename, | ||
| sourceMap, | ||
| line, | ||
| col, | ||
| enclosingLine, | ||
| enclosingCol, | ||
| env, | ||
| action, |
There was a problem hiding this comment.
We want the fake function to be generated based on both like we do in ReactFlightClient.
Right now this causes devtools to point to the beginning of the module since that's the enclosing line number of the registerServerReference call.
CleanShot.2025-05-07.at.12.39.37.mp4
Before this change we pointed at the declaration of increment since that's where registerServerReference call sourcemaps to:
1 | 'use server'
^ maps to 'use server'
enclosing line number
we don't want that
... |
29 | registerServerReference(increment, "...", "increment");
^ maps maps to `function increment() {}`
line number
we want that
There was a problem hiding this comment.
I guess we never want the enclosing because the line in the location field always represents the enclosing, which is the only thing we encode here.
There was a problem hiding this comment.
Reverted that part.
There was a problem hiding this comment.
We don't need it for this specific case I believe but generally we'd still want the enclosing number for the case where bundlers move modules into functions that their runtime executes.
We already have this problem today where these functions pollute the stackframes of errors that happen during module evaluation e.g. at [app-rsc] [project]/app/page.tsx (app/page.tsx:1:2). We want the correct enclosing line number for that case as well so that we can map these function names to something appropriate i.e. no name so that the stackframe just ends up as at app/page.tsx:1:2.
Though one insight here is that for Server functions we never want to use the enclosing scope for name mapping but the actual location. The enclosing scope would map to the module when we want the name at the callsite. But I guess we're not even using sourcemaps to get the name today and instead use the transported original function name directly.
There was a problem hiding this comment.
There's never anything that executes inside the function so there's never anything that gets the name from source mapping. But if for example printing it in the console printed the name from the source mapped source then it would source map it but even then it would be correct because it is the enclosing line that we're modeling in the fake function. The registerServerReference is at the enclosing location and its callsite location is therefore modeling the enclosing location which we are then modeling with the fake function's enclosing location.
We just have to be careful if we do something like extract the enclosing and passing that as "location" meta data. I think maybe that's the modeling issue here. The server should actually encode the location as the enclosing line.
If we don't have the ability to get enclosing line/column we encode it as zero. This will point to the beginning of the file which is likely to cause a miss in source mapping to an identifier which is better since it falls back to getting the name of the function in that case instead.
… fake function (#33136) Stacked on #33135. This encodes the line/column of the enclosing function as part of the stack traces. When that information is available. I adjusted the fake function code generation so that the beginning of the arrow function aligns with these as much as possible. This ensures that when the browser tries to look up the line/column of the enclosing function, such as for getting the function name, it gets the right one. If we can't get the enclosing line/column, then we encode it at the beginning of the file. This is likely to get a miss in the source map identifiers, which means that the function name gets extracted from the runtime name instead which is better. Another thing where this is used is the in the Performance Track. Ideally that would be fixed by https://issues.chromium.org/u/1/issues/415968771 but the enclosing information is useful for other things like the function name resolution anyway. We can also use this for the "View source for this element" in React DevTools. DiffTrain build for [4a70286](4a70286)
…Site (#33141) Follow up to #33136. This clarifies in the types where the conversion happens from a CallSite which we use to simulate getting the enclosing line/col to a FunctionLocation which doesn't represent a CallSite but actually just the function which only has an enclosing line/col. DiffTrain build for [a437c99](a437c99)
…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.
…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)
…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.
Stacked on #33135.
This encodes the line/column of the enclosing function as part of the stack traces. When that information is available.
I adjusted the fake function code generation so that the beginning of the arrow function aligns with these as much as possible.
This ensures that when the browser tries to look up the line/column of the enclosing function, such as for getting the function name, it gets the right one. If we can't get the enclosing line/column, then we encode it at the beginning of the file. This is likely to get a miss in the source map identifiers, which means that the function name gets extracted from the runtime name instead which is better.
Another thing where this is used is the in the Performance Track. Ideally that would be fixed by https://issues.chromium.org/u/1/issues/415968771 but the enclosing information is useful for other things like the function name resolution anyway.
We can also use this for the "View source for this element" in React DevTools.