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

Reimplement using Babel plugins #259

Merged
merged 26 commits into from Nov 30, 2016
Merged

Reimplement using Babel plugins #259

merged 26 commits into from Nov 30, 2016

Conversation

benjamn
Copy link
Collaborator

@benjamn benjamn commented Nov 21, 2016

Supersedes #222. Thanks for getting that started, @kittens, and I'm glad it's finally (almost) done!

With this pull request, Regenerator now consists of three helper packages:

  • regenerator-runtime, which implements various runtime functions relied upon by generated code. Already used by Babel, with an upgrade pending.
  • regenerator-transform, a pure-Babel implementation of the Regenerator AST transform, suitable for independent use by any Babel compilation pipeline. Intended to replace and/or become the implementation of babel-plugin-transform-regenerator.
  • regenerator-preset, a group of Babel plugins that enforce various preconditions of the Regenerator transform, such as transformation of block-scoped let and const declarations. See the full list here.

Why move the implementation of Regenerator back into this repository?

  1. Test coverage. Believe it or not, the Babel version doesn't run any of the numerous tests from this repository, which inhibits safe forward development of Regenerator within the Babel repository.
  2. A place to file and fix bugs. I would love to spend time investigating and fixing the issues that have been reported against this repository, and I believe this repository is a better place to track Regenerator bugs than https://github.com/babel/babel/issues, but that effort only makes sense for me if this repository represents the source code of Regenerator.
  3. Babel ecosystem compatibility. Regenerator currently relies on various tools that have been deprecated and/or reimplemented by Babel, such as the defs compiler for block-scoped declarations, and my ad-hoc implementation of for-of loops. Reimplementing Regenerator as a Babel plugin allows it to take full advantage of Babel's highly-optimized plugin toolchain, without pulling in any unnecessary dependencies like recast and ast-types.

In case you're not aware, the Babel implementation was originally adapted from this repository, which is perfectly fine according to the LICENSE, but the vast majority of meaningful commits in the history of this project still reside in this repository, so I believe this is where Regenerator ought to live.

cc @hzoo @loganfsmyth et al.

Babylon requires `await yield x` to be parenthesized in `async` generator
functions: `await (yield x)`. Easy.

Babel uses closures to enclose block-scoped function declarations, and if
those closures contain yield expressions, then a nested delegate generator
gets created, with its own scope for variable declarations. This
difference in behavior caused the `halve` function to become inaccessible
where I was calling it before, which is arguably more correct, so I've
removed the brittle part of that test.
Note that this test still runs and passes in native Node.
@benjamn
Copy link
Collaborator Author

benjamn commented Nov 21, 2016

Since there are a bunch of commits here, this commit is the real meat of this pull request.

@benjamn
Copy link
Collaborator Author

benjamn commented Nov 21, 2016

Now that Regenerator is a multi-package project, I would love some guidance about using Lerna to wrangle the helper packages.

This is important so that CI tests can catch errors whenever a change is
pushed for a helper package, without waiting for that (possibly broken)
helper package to be published again.
benjamn added a commit to benjamn/babel that referenced this pull request Nov 21, 2016
…ator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.5). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.
This is a test-only fix, since most projects that care about supporting
Node 0.10 and/or older browsers will use babel-plugin-transform-runtime to
polyfill APIs like Symbol.
These dependencies were introduced by 8cdae42.
It's frustrating how easy it is to mess up your dependencies with
`npm upgrade --save[-dev]`.
@benjamn benjamn merged commit 3c1bd55 into master Nov 30, 2016
@benjamn
Copy link
Collaborator Author

benjamn commented Nov 30, 2016

As there have been no objections, I've gone ahead and merged this. I will obviously bump the minor version before publishing!

benjamn added a commit that referenced this pull request Nov 30, 2016
The minor version bump is mostly due to the merging of #259.
benjamn added a commit that referenced this pull request Nov 30, 2016
The minor version bump is mostly due to the merging of #259.
benjamn added a commit to benjamn/babel that referenced this pull request Dec 1, 2016
…ator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.5). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.
benjamn added a commit to benjamn/babel that referenced this pull request Dec 1, 2016
…ator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.7). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.
benjamn added a commit to benjamn/babel that referenced this pull request Dec 1, 2016
…ator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.7). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.
hzoo pushed a commit to babel/babel that referenced this pull request Dec 8, 2016
…ator (#4881)

* Use regenerator-transform to implement babel-plugin-transform-regenerator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.7). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.

* Remove never-used babel-plugin-transform-regenerator/.test directory.

* regenerator-transform to 0.9.8
panagosg7 pushed a commit to panagosg7/babel that referenced this pull request Jan 17, 2017
…ator (babel#4881)

* Use regenerator-transform to implement babel-plugin-transform-regenerator.

Though I believe this is safe to merge, you may want to wait to merge it
until facebook/regenerator#259 is merged. That
pull request also provides more explanation of why I think moving the
implementation of Regenerator back to its original repository is a good
idea. I'm happy to compromise on anything necessary to make this happen.

Note that the regenerator-transform version is pegged to an exact version
(0.9.7). Past experience has taught me that releasing new versions of
Regenerator is almost impossible if client projects are allowed to pull in
new patch versions automatically. I will happily submit a pull request
whenever a new version is released.

* Remove never-used babel-plugin-transform-regenerator/.test directory.

* regenerator-transform to 0.9.8
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.

None yet

2 participants