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

ship @babel/standalone source maps #14863

Merged
merged 5 commits into from Aug 31, 2022

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Aug 18, 2022

Q                       A
Fixed Issues? Address #14179 (comment)
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

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.

@JLHwung JLHwung added PR: Internal 🏠 A type of pull request used for our changelog categories pkg: standalone labels Aug 18, 2022
@@ -264,6 +264,7 @@ target["prepublish-build"] = function () {
target["prepublish-prepare-dts"]();
},
{
NODE_ENV: "production",
Copy link
Member

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.

Copy link
Member

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? 🙏

Copy link
Member

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.

@liuxingbaoyu
Copy link
Member

Also maybe we can generate all source maps in development environment? That should make debugging easier for us.

@JLHwung JLHwung force-pushed the ship-babel-standalone-source-map branch from 9a96668 to ed50805 Compare August 18, 2022 14:35
@babel-bot
Copy link
Collaborator

babel-bot commented Aug 18, 2022

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/52850/

@JLHwung
Copy link
Contributor Author

JLHwung commented Aug 18, 2022

Also maybe we can generate all source maps in development environment?

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 sourcemaps: true to babel-worker.cjs.

@JLHwung JLHwung force-pushed the ship-babel-standalone-source-map branch from ed50805 to 004a010 Compare August 18, 2022 14:44
@JLHwung
Copy link
Contributor Author

JLHwung commented Aug 18, 2022

Hmm I have no idea why GitHub CI is not showing up. I will re-invoke the CI an hour later.

@liuxingbaoyu
Copy link
Member

https://www.githubstatus.com/incidents/8gpymn7rystj
lol

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Aug 18, 2022

CI is green! Can you modify this for repl by the way?

- &artifact_babel
path: ~/babel/packages/babel-standalone/babel.js
- &artifact_babel_min
path: ~/babel/packages/babel-standalone/babel.min.js

I'm not quite sure how to get repl to support source maps.😕

@JLHwung
Copy link
Contributor Author

JLHwung commented Aug 18, 2022

The browser should work out of the box as long as source maps are served. I will give it a try.

@liuxingbaoyu
Copy link
Member

Now that the source map is working, maybe you need to enable this in your browser.💐
https://umaar.com/dev-tips/96-resolve-variable-names/

Also interesting is that the previous failure was wrong with NODE_ENV, I think I might open a PR in the future to unify this to BABEL_ENV.

@JLHwung
Copy link
Contributor Author

JLHwung commented Aug 29, 2022

@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

@liuxingbaoyu
Copy link
Member

liuxingbaoyu commented Aug 29, 2022

Oh, you're right, I actually enabled this as well.
Since my devtools is in Chinese, I didn't find what the article said, so I finally found it by searching for var in the experimental feature.😄

@nicolo-ribaudo nicolo-ribaudo force-pushed the ship-babel-standalone-source-map branch from d60a24e to 6ce0dba Compare August 31, 2022 20:15
@nicolo-ribaudo nicolo-ribaudo merged commit 35a9df0 into babel:main Aug 31, 2022
@nicolo-ribaudo nicolo-ribaudo deleted the ship-babel-standalone-source-map branch August 31, 2022 20:24
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 1, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: standalone PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants