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

Convert to a Babel plugin #222

Closed
wants to merge 1 commit into from
Closed

Convert to a Babel plugin #222

wants to merge 1 commit into from

Conversation

sebmck
Copy link
Contributor

@sebmck sebmck commented Oct 5, 2015

This is WIP. There are currently ~8 failing tests so I thought I'd put it up for review before I look into them too much. This will heavily improve Babel performance and stability. I know there are ways for regenerator to perform it's compilation in a single pass in the future too (right now there are a few visitors that are ran independently).

NOTE: This wont currently run unless you've got a development branch locally checked out and everything linked which is a bit of a PITA.

AFAIK this was 👍 between @benjamn and @amasad.

TODO:

  • Add back includeRuntime option.
  • Add back regenerator.transform API.

@benjamn
Copy link
Collaborator

benjamn commented Oct 5, 2015

Great, glad to have a chance to look this over sooner rather than later.

Two first thoughts:

  • Is there anything you can do to get the Travis tests to run, even with failures? Right now they're just erroring out at npm install time.
  • Is it going to be possible to provide a require("regenerator").transform function that operates on an AST object, rather than taking in a string and returning a string?

exports.compile = compile;

// To modify an AST directly, call require("regenerator").transform(ast).
exports.transform = transform;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Removing the require("regenerator").transform API is a pretty big breaking change for any non-Babel consumers of this library.

@sebmck
Copy link
Contributor Author

sebmck commented Oct 5, 2015

@benjamn

  1. Is there anything you can do to get the Travis tests to run, even with failures? Right now they're just erroring out at npm install time.

I'll get out a release candidate ASAP so Travis at least shows the errors and you can run this locally without mucking about.

  1. Is it going to be possible to provide a require("regenerator").transform function that operates on an AST object, rather than taking in a string and returning a string?

Yeah. Shouldn't be a problem. I'll add a TODO list.

@sebmck
Copy link
Contributor Author

sebmck commented Mar 10, 2016

cc @benjamn. What's required to move this forward? Would like to get this merged ASAP.

@benjamn
Copy link
Collaborator

benjamn commented Nov 30, 2016

Implemented by #259.

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.

3 participants