FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid `require()` call in `@babel/standalone` bundle by nicolo-ribaudo · Pull Request #16639 · babel/babel · GitHub

/ babel Public

Avoid require() call in @babel/standalone bundle - #16639

Merged
nicolo-ribaudo merged 5 commits into
babel:mainfrom
nicolo-ribaudo:fix-16634
Jul 15, 2024
Merged

nicolo-ribaudo merged 5 commits into
babel:mainfrom
nicolo-ribaudo:fix-16634

Conversation

Copy link
Copy Markdown
Member
Q                       A
Fixed Issues? Fixes #16634
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

I'm not sure about how to test this, since it's about code there just for backwards compat and not actually used. It just being there unused causes problems for bundlers.

The solution is that we must not have require calls in ESM files unless they are removed in the IS_STANDALONE bundle.

nicolo-ribaudo added PR: Bug Fix 🐛 A type of pull request used for our changelog categories i: regression pkg: standalone labels Jul 12, 2024

babel-bot commented Jul 12, 2024
edited
Loading

Copy link
Copy Markdown
Collaborator

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

JLHwung left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

If we want to make sure it never happens again, we can use acorn-walk to visit the AST and then throw on require() calls:

describe("@babel/standalone", () => {
it("should be built into ES5", () => {
const babelStandaloneSource = fs.readFileSync(
require.resolve("../babel.js"),
{ encoding: "utf8" },
);
expect(() => {
acornParse(babelStandaloneSource, {
ecmaVersion: 5,
sourceType: "script",
});
}).not.toThrow();

Copy link
Copy Markdown
Member Author

I added a simple test checking the number of require() calls, so that if it changes we notice and can manually review.

Copy link
Copy Markdown
Member Author

It's getting late here, but I'll merge and release this tomorrow morning :)

This comment was marked as outdated.

liuxingbaoyu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I see, I can reproduce it only when make use-cjs.

nicolo-ribaudo and others added 2 commits July 13, 2024 06:59
Co-authored-by: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
github-actions Bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 15, 2024
github-actions Bot locked as resolved and limited conversation to collaborators Oct 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

i: regression outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: standalone PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: errors importing @babel/standalone since yesterdays Babel release

4 participants


Back | FazBrowse Home | New Git URL