Skip to content
This repository was archived by the owner on Feb 1, 2025. It is now read-only.

Convert to a Babel plugin#222

Closed
sebmck wants to merge 1 commit intofacebook:masterfrom
babel:babelify
Closed

Convert to a Babel plugin#222
sebmck wants to merge 1 commit intofacebook:masterfrom
babel:babelify

Conversation

@sebmck
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

benjamn commented Nov 30, 2016

Implemented by #259.

@benjamn benjamn closed this Nov 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants