speedskater / babel-plugin-rewire

A babel plugin adding the ability to rewire module dependencies. This enables to mock modules for testing purposes.
843 stars 90 forks source link

Babel 7 Support? #209

Closed damassi closed 5 years ago

damassi commented 6 years ago

I'm wondering if you have any plans to support Babel 7?

speedskater commented 6 years ago

@damassi Sorry for having not worked on babel-plugin-rewire over the last months. We are definitely planning to upgrade to babel 7, as we are heavily relying on the plugin ouselves. As we are currently having a tight schedule we are delaying this transition, but the transition to babel 7 is already planned. In case the final version of babel 7 will be released before that, we will start the transition as soon as possible.

damassi commented 6 years ago

Thanks @speedskater 👍 FWIW I tried quite hard to figure out how to fix in this repo https://github.com/damassi/babel-7-plugin-rewire and was able to get the unit tests passing (after running through Prettier and Uglify (without mangle) to account for differences in output between Babel 6 and Babel 7, but there's something up with executing the plugin at runtime (as can be seen by attempting to run the usage tests, or attempting to use babel-node). When running through babel for straight compiled file output everything works great.

NilSet commented 6 years ago

@loganfsmyth helped fix the bug preventing this from working in babel 7. A missing param to t.Program was making babel treat a file as a commonjs module when it was actually an es6 module. I will put up a pr based on @damassi 's work shortly.

loganfsmyth commented 6 years ago

Yeah I can't speak to your branch specifically @damassi, but it shouldn't be necessary to for this plugin to drop support for 6 in order to support 7.x. If you'd like to use 7.x for your actual builds that's pretty reasonable, but it isn't necessary for the plugin itself to work on 7.x.

I'd be happy to review or give pointers if there are any more questions.

@NilSet I wouldn't expect conflicts, so I'd probably say just make your PR based on master rather than trying to take @damassi's work into account.

damassi commented 6 years ago

Yeah my PR changes a lot of stuff :) Its quite different than whats in the repo as I thought we were going to have to own a fork for a while. That said, since there is so much copy / paste around user issues I think some work will be needed around formatting inputs for tests, similar to what I have in my branch.

@NilSet or @loganfsmyth - care to share what the missing param change was? I'm so curious as I spent quite a lot of time trying to figure this out.

loganfsmyth commented 6 years ago

t.Program here https://github.com/speedskater/babel-plugin-rewire/blob/9f40904a4333a6f38825837c2108a97afcd7c016/src/babel-plugin-rewire.js#L203 takes 3 parameters, and only 2 are being passed, which essentially converts the file from ESM to CommonJS as far as Babel is concerned, which means if the user is trying to compile ESM to CommonJS to run on Node, it won't ever run because it'll think the conversion already happened.

There might be other issues, but that was the one we tracked down. Node was throwing

Unexpected token 'import'

at the first import it hit.

damassi commented 6 years ago

Yup, that was the issue I ran into. Thanks for looking at this! In the end we ended up swapping out babel-plugin-rewire with node-rewire and updating tests.

sojungko commented 6 years ago

Running into the same issue as @loganfsmyth . I tried forking @NilSet 's PR that supposedly fixes it (I built it locally and linked it to my repo) but it still throws the Unexpected token 'import' error.

wolfy1339 commented 5 years ago

This is required by babel-plugin-module-exports which in turn is required by babel-plugin-dynamic-import, which is required by CRA.