Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for sourcemap names #1296

Closed
connor4312 opened this issue May 19, 2021 · 10 comments · Fixed by #2415
Closed

Support for sourcemap names #1296

connor4312 opened this issue May 19, 2021 · 10 comments · Fixed by #2415

Comments

@connor4312
Copy link

connor4312 commented May 19, 2021

Chrome has some support for renamed identifiers in sourcemaps. Now, VS Code's debugger does too. It looks like esbuild doesn't generate names, but if it did it would make debugging easier.

@evanw
Copy link
Owner

evanw commented May 26, 2021

Cool! I originally hadn't implemented this because I couldn't find a case where it mattered. I'm glad to hear that tooling has finally caught up. In addition to esbuild generating these names itself, it will also have to map names forward from embedded source maps in input files.

@kzc
Copy link
Contributor

kzc commented Jun 21, 2021

source-map-support has been used in NodeJS CLI tooling for stack traces since the early days and it makes use of sourcemap "names". I had a stack trace discrepancy with a node CLI tool that was built with esbuild which led me to this issue.

Example CLI program:

$ cat names.js
try {
    require("source-map-support").install();
} catch (err) {}
Error.stackTraceLimit = 3; // not needed - demo purposes only
function someReallyDescriptiveFunc(flag) {
    if (flag) secondTime(flag);
}
function secondTime(flag) {
    if (flag) lastCall(flag);
}
function lastCall(flag) {
    if (flag) console.log(new Error("sourcemap names demo").stack);
}
someReallyDescriptiveFunc(0);
someReallyDescriptiveFunc(1);

Expected output without a sourcemap:

$ node names.js 
Error: sourcemap names demo
    at lastCall (/testing/names.js:12:27)
    at secondTime (/testing/names.js:9:15)
    at someReallyDescriptiveFunc (/testing/names.js:6:15)

Output with esbuild with symbol mangling and a sourcemap:

$ esbuild names.js --bundle --minify-identifiers --minify-whitespace --sourcemap --platform=node --external:source-map-support --outfile=names.esbuild.js --log-level=warning && node names.esbuild.js
Error: sourcemap names demo
    at o (/testing/names.js:12:27)
    at c (/testing/names.js:9:15)
    at r (/testing/names.js:6:15)

Output with rollup+terser with symbol mangling and a sourcemap:

$ rollup names.js --external=source-map-support --sourcemap -o names.rollup.js -p terser='{"compress":false}' --silent && node names.rollup.js
Error: sourcemap names demo
    at lastCall (/testing/names.js:12:27)
    at secondTime (/testing/names.js:9:15)
    at r (/testing/names.js:6:15)

The same output using terser alone with symbol mangling and a sourcemap:

$ terser names.js --source-map "includeSources=true,url='names.terser.js.map'" --toplevel --mangle -o names.terser.js && node names.terser.js
Error: sourcemap names demo
    at lastCall (/testing/names.js:12:27)
    at secondTime (/testing/names.js:9:15)
    at r (/testing/names.js:6:15)

source-map-support usually works quite well, but it appears to have a bug generating the name someReallyDescriptiveFunc. This bug is not present in various online source map visualizers.

Mangling symbols is not common in bundling NodeJS CLI tools, it was just used to demonstrate the issue more clearly. The actual discrepancies in my case were seeing things renamed like Identifier2 instead of Identifier.

@kzc
Copy link
Contributor

kzc commented Jun 21, 2021

Follow up...

If the following line in the names.js example above is removed:

Error.stackTraceLimit = 3; // not needed - demo purposes only

then source-map-support will correctly emit the stack frame for someReallyDescriptiveFunc for both rollup+terser and terser alone. It was just a NodeJS stack frame edge case, and the sourcemap "names" generation was fine.

@rdrabik
Copy link

rdrabik commented Jul 8, 2022

Hey @evanw, are there any plans to add support for the source map names in the near future?

@evanw
Copy link
Owner

evanw commented Jul 25, 2022

I have implemented support for names generation and I believe that the implementation works as intended. You can see an example of esbuild's output here (hover over identifiers on the right to see the original names).

However, I have not used the feature myself yet as I don't typically use debuggers. Please evaluate esbuild's implementation and let me know if there are any issues.

@bpasero
Copy link

bpasero commented Aug 30, 2022

I gave this a spin from a branch applying esbuild@0.15.5 for VSCode (commit). We have a rather simple esbuild invocation: https://github.com/microsoft/vscode/blob/a13e5e1da6d87e4dac3bfd9e61145f67dec29a43/build/lib/optimize.ts#L291-L299

The resulting build is not allowing me to quick outline into the methods so I am not 100% sure if it is working or if something is missing.

An example source map with this change is: https://ticino.blob.core.windows.net/sourcemaps/8bd968242f169d3c6a84c19bad1ac78d0b3441c4/core/vs/workbench/workbench.desktop.main.js.map

An example VSCode exploration build from this branch is: https://az764295.vo.msecnd.net/exploration/8bd968242f169d3c6a84c19bad1ac78d0b3441c4/VSCode-darwin-universal.zip

@evanw
Copy link
Owner

evanw commented Aug 30, 2022

I visualized that source map here and it looks like the names are attached correctly: https://evanw.github.io/source-map-visualization/.

@bpasero
Copy link

bpasero commented Aug 30, 2022

Hm, I am mainly referring to the resolution in https://bugs.chromium.org/p/chromium/issues/detail?id=1087763#c15 which states we have to update to latest esbuild to see variable names in devtools. Our VSCode devtools is on Chrome 102, which I would assume is good enough for this.

@evanw
Copy link
Owner

evanw commented Aug 30, 2022

Ah. Chrome 102 through 104 has a bug with reading the source maps that esbuild generates: https://bugs.chromium.org/p/chromium/issues/detail?id=1329997. I hear this has been fixed in Chrome 105 which is being released today? According to #2295 (comment).

@bpasero
Copy link

bpasero commented Aug 30, 2022

@evanw oh wow thanks! I did an experiment with using devtools in the browser for the sources (Chrome 104) and in fact I do see a difference.

Old
image

New
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants