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

Jump to bottom

Add cloneInputAst option to babel.transformFromAst #10241

Merged
merged 1 commit into from Jul 29, 2020
Merged

Add cloneInputAst option to babel.transformFromAst #10241

merged 1 commit into from Jul 29, 2020

Conversation

coderaiser
Copy link
Contributor

@coderaiser coderaiser commented Jul 19, 2019 •
edited by JLHwung

Q                       A
Fixed Issues? Fixes #10231, fixes #11206
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature? Added new flag cloneInputAst
Tests Added + Pass? Yes
Documentation PR Link babel/website#2306
Any Dependency Changes?
License MIT

@nicolo-ribaudo nicolo-ribaudo added the PR: New Feature 🚀 A type of pull request used for our changelog categories label Jul 19, 2019
nicolo-ribaudo
nicolo-ribaudo reviewed Jul 19, 2019
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

I'm not a fan of this name, since it doesn't tell what it is copying. Maybe cloneInputAst?

@babel-bot
Copy link
Collaborator

babel-bot commented Jul 19, 2019 •
edited

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

@coderaiser
Copy link
Contributor Author

coderaiser commented Jul 19, 2019 •
edited

That’s a good idea, cloneInputAst feats more 🙂.
Just pushed changes.

nicolo-ribaudo
nicolo-ribaudo approved these changes Apr 29, 2020
View reviewed changes
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo left a comment

Choose a reason for hiding this comment

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

This PR makes it way easier to write codemods using recast (which keepts the original formatting). If we clone the AST, then recast will see all the output AST nodes as new and completely discard the original source format.

Without this PR, the only way to make it work is to inject the input AST in a pre or Program visitor: https://github.com/nicolo-ribaudo/codemods/blob/40e90e3ad6e2db91fd7756bcce68bfefd373a3a3/src/utils.js#L32-L68

@nicolo-ribaudo nicolo-ribaudo changed the title feature: babel-core: add deepCopy flag (#10231) Add cloneInputAst option to babel.transformFromAst Apr 29, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Apr 29, 2020 •
edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8983fe7:

Sandbox Source
exciting-golick-w2ui3 Configuration
great-wood-e9jj0 Configuration

@nicolo-ribaudo
Copy link
Member

@coderaiser I have rebased this PR, since it looks like there were some problems after merging master into this branch.

@coderaiser
Copy link
Contributor Author

@nicolo-ribaudo looks like something wrong on windows with node v14 in places that not related to this PR.

@nicolo-ribaudo
Copy link
Member

It's a node bug, we are investigating it.

@coderaiser
Copy link
Contributor Author

coderaiser commented Jun 2, 2020 •
edited

@nicolo-ribaudo Any progress on this :)?

coderaiser added a commit to coderaiser/putout that referenced this pull request Jun 3, 2020
@coderaiser
chore(@putout/engine-loader) transforms: babel: deepCopy -> clonseInp…
faccda1 
…utAst (babel/babel#10241)
@nicolo-ribaudo nicolo-ribaudo added this to the 7.11.0 milestone Jun 6, 2020
@JLHwung JLHwung changed the base branch from master to main June 24, 2020 19:24
JLHwung
JLHwung previously approved these changes Jun 24, 2020
View reviewed changes
@JLHwung
Copy link
Contributor

JLHwung commented Jun 24, 2020

@coderaiser Can you rebase on main?

@JLHwung JLHwung added the PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release label Jun 24, 2020
@coderaiser
Copy link
Contributor Author

@JLHwung done :)

@JLHwung JLHwung dismissed their stale review June 24, 2020 19:57

CI error is related.

JLHwung
JLHwung approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Thanks.

@coderaiser
Copy link
Contributor Author

coderaiser commented Jul 5, 2020 •
edited

test262 takes too long, looks like they should be approved.

@nicolo-ribaudo
Copy link
Member

Yeah sorry, we don't run test262 on PRs automatically since often it's not needed. We haven't merged this PR yet just because we are waiting for the next minor release.

@JLHwung JLHwung merged commit cf425a0 into babel:main Jul 29, 2020
@coderaiser coderaiser deleted the feature/babel-core-deep-copy branch July 30, 2020 05:14
@nicoespeon nicoespeon mentioned this pull request Sep 1, 2020
Merged
@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 Oct 30, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@xtuc xtuc xtuc approved these changes

@JLHwung JLHwung JLHwung approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

@kaicataldo kaicataldo kaicataldo approved these changes

Assignees
No one assigned
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: New Feature 🚀 A type of pull request used for our changelog categories PR: Ready to be Merged A pull request with already two approvals, but waiting for the next minor release
Projects
None yet
Milestone
7.11.0
6 participants
@coderaiser @babel-bot @nicolo-ribaudo @JLHwung @xtuc @kaicataldo