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

Move all runtime helpers to individual files #16495

Merged
merged 10 commits into from
May 16, 2024

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented May 15, 2024

Q                       A
Fixed Issues? Fixes #1, Fixes #2
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

After this PR, since there is a build step for all helpers, I plan to move

function getHelperMetadata(file: File): HelperMetadata {
to be done at build-time rather than at runtime.

This PR caught a problem with our Babel7/8 integration tests: some tests fail because the helpers are missing (for example, old decorator helpers), but we didn't notice because we where generating those helpers even in Babel 8 mode during the test.

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label May 15, 2024
@@ -26,6 +26,15 @@
"./helpers/AsyncGenerator.js"
],
"./helpers/esm/AsyncGenerator": "./helpers/esm/AsyncGenerator.js",
"./helpers/AwaitValue": [
Copy link
Member Author

Choose a reason for hiding this comment

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

These package.json files are now just sorted alphabetically, but the contents are the same.

@babel-bot
Copy link
Collaborator

babel-bot commented May 15, 2024

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

@nicolo-ribaudo nicolo-ribaudo force-pushed the helpers-refactor branch 2 times, most recently from cf1de47 to 624ef72 Compare May 15, 2024 15:55
Copy link
Member

@liuxingbaoyu liuxingbaoyu left a comment

Choose a reason for hiding this comment

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

Maybe it would be better to keep Object.freeze(helpers)?

@nicolo-ribaudo
Copy link
Member Author

In tests we mutate the helpers object to add fake helpers, so unfortunately it doesn't work. The final helpers object was already unfrozen, because we spread the frozen object into a non-frozen one.

@nicolo-ribaudo
Copy link
Member Author

I'll open a good first issue to convert the helpers to TS (in chunks)

@nicolo-ribaudo nicolo-ribaudo merged commit db3e9a6 into babel:main May 16, 2024
51 checks passed
@nicolo-ribaudo nicolo-ribaudo deleted the helpers-refactor branch May 16, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: helpers 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