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

Allow transform to be a promise. Resolves #210. #237

Merged
merged 1 commit into from
Mar 31, 2021

Conversation

rektide
Copy link
Contributor

@rektide rektide commented Feb 6, 2018

Hello. Wanted to float this, as a way to allow transforms to be async. This unlocks any kind of transform that would want to query some exterior piece of state.

@rektide
Copy link
Contributor Author

rektide commented Feb 11, 2018

Test failures are on Node v4. This is expected, as v4 does not have async function support.

It is worth noting that Node v4 is end of life in two months.

@ChiefORZ
Copy link

ChiefORZ commented May 9, 2019

any updates on merging this PR?

1 similar comment
@maksimsemenov
Copy link

any updates on merging this PR?

@Daniel15
Copy link
Member

Daniel15 commented Jun 30, 2020

Does this have a performance impact for non-async transforms, if they're running across tens of thousands of files?

This seems safe to me, given jscodeshift now has a minimum of Node.js 8.x which comes with async/await enabled out-of-the-box, I'm just curious as to if it has a noticeable perf impact.

@Daniel15 Daniel15 mentioned this pull request Jun 30, 2020
@rektide
Copy link
Contributor Author

rektide commented Jul 2, 2020

I don't have any good stress tests to benchmark. :( There will be some impact, yes, but whether it's perceptible is hard to say.

@NickHeiner NickHeiner mentioned this pull request Sep 14, 2020
@foush
Copy link

foush commented Mar 3, 2021

@Daniel15 any chance we can consider merging this? Would be nice to have async support for more complex codemods

@Daniel15
Copy link
Member

Let's do it!

@Daniel15 Daniel15 merged commit d8fb5ec into facebook:master Mar 31, 2021
@Daniel15
Copy link
Member

I'll publish this to npm soon, I just need to figure out who has permission to publish the package. Sorry for the delay @rektide!

@Daniel15
Copy link
Member

Published as 0.12.0. Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants