-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
ship @babel/standalone source maps #14863
ship @babel/standalone source maps #14863
Conversation
@@ -264,6 +264,7 @@ target["prepublish-build"] = function () { | |||
target["prepublish-prepare-dts"](); | |||
}, | |||
{ | |||
NODE_ENV: "production", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we unify NODE_ENV
into BABEL_ENV
? There is currently only one place where NODE_ENV
is used, and there are more BABEL_ENV
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you open a PR to replace both? 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I'll do this after 7.19.0
is released, it's a little messy and I'm worried about causing accidents.
Also maybe we can generate all source maps in development environment? That should make debugging easier for us. |
9a96668
to
ed50805
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52850/ |
Personally I am cool with that because few syntaxes are transformed when I am developing on latest Node.js. With that said, I would not block such PR to add |
ed50805
to
004a010
Compare
Hmm I have no idea why GitHub CI is not showing up. I will re-invoke the CI an hour later. |
CI is green! Can you modify this for repl by the way? Lines 12 to 16 in 81158a5
I'm not quite sure how to get repl to support source maps.😕 |
The browser should work out of the box as long as source maps are served. I will give it a try. |
Now that the source map is working, maybe you need to enable this in your browser.💐 Also interesting is that the previous failure was wrong with |
@liuxingbaoyu Thanks for the update. I don't find "Resolve variable names" in my devtools settings because it has been enabled by default. There are a new flag "Resolve variable names in expressions" though. See also https://bugs.chromium.org/p/chromium/issues/detail?id=327092 |
Oh, you're right, I actually enabled this as well. |
d60a24e
to
6ce0dba
Compare
Ship
@babel/standalone
source maps. Previously when@babel/standalone
throws an error, the stack trace is barely readable due to minifying. Shipping the source maps and if a website owner hosts such maps, the situation should be improved.Of course, the source maps will increase the published artifact size. But given that
@babel/standalone
is meant for browser users. I think it is acceptable.