polyfill-regenerator doesn't support targets

[agilgur5]

Issue

Hey there, so I've been trying to add this to TSDX in jaredpalmer/tsdx#795 but ran into an issue: jaredpalmer/tsdx#795 (comment). Long story short, polyfill-regenerator doesn't seem to support targets.

If I'm understanding correctly, this line needs to specify targets per the polyfill-provider docs.

Would be happy to write a PR for this and will look into it more tomorrow. I'm not sure if I should just manually input the data from MDN or if I should use babel-compat-data for transform-regenerator? Any guidance would be appreciated

Context

Per above, was looking to get this to work with TSDX, which would add quite a lot of indirect library users of this.

Concretely, I was adding this to replace the now defunct/unmaintained babel-plugin-async-to-promises (that has some correctness bugs) without the downsides of an impure preset-env useBuiltIns or all-or-nothing transform-runtime.

P.S.

Also just wanted to say really happy to see this set of libraries existing to provide a solution for gaps between preset-env useBuiltIns and transform-runtime, solves a long existing problem!

[nicolo-ribaudo]

I didn't implement targets support for polyfill-regenerator because I don't think it would give any advantage. The regenerator-runtime library is necessary only and always when you compile generators.

@babel/plugin-transform-regenerator injects a call to a global regeneratorRuntime variable, and polyfill-regenerator detects this injected variable usage and injects the polyfill when it's present.

This means that, even if you always enable the polyfill plugin, it only does something when you transpile generators.

I think that allowing targets for polyfill-regenerator can only cause problems, in case the targets or compat data of the transform-regenerator and polyfill-regenerator get out of sync.

However, maybe we could throw a better error message 🤔

[agilgur5]

Ohhhh gotcha, so it'll only run if transform-regenerator has already run basically, so only have to configure preset-env's targets.

That's great, then I don't need to duplicate targets either! Will update my tests to account for that. Thanks for the quick and informative response 🙂

Yea I think a warning logged out or error thrown if targets is used would be helpful. Right now it doesn't log or error at all, basically just silently ignores targets. A small note on this in the docs would be useful too since this behaves differently / doesn't need targets duplication unlike the other polyfill plugins.

I can add a PR for both of those and get your feedback

[nicolo-ribaudo]

Sounds good to me, feel free to open a PR for both!

[agilgur5]

Finally made some time for this and wrote up a PR to throw an error in #119 !

I didn't change the docs as targets have been removed from most options as of 1091fa4 and my PR #116 , so I thought that clarification would no longer be necessary