Ability to ignore files that are already ES modules

[tbranyen]

Ability to ignore files that are already ES modules

[tbranyen]

I fixed this in 1.0.0. Can you give it a try and let me know if it works as you'd expect?

[tbranyen]

Hey @coreyfarrell good that you have a workaround using the overrides option. Do you have a proposal for how we should check if something is CJS or not? I could do require/module/exports sniffing, or check the sourceType, but perhaps what you have is fine, just expand it a bit more to:

test: [path.resolve(__dirname, '../node_modules/**')],
plugins: ['transform-commonjs']

This way all node_modules get affected. I'm open to other ideas though on how to support this feature request.

[coreyfarrell]

Thanks for the response. I'm unable to expand the override test to all node_modules as most that I use from web pages are already ES modules (camelcase is currently the only exception).

I don't have a good proposal for checking if something is CJS. I think looking for module.exports or exports variables would probably be best, only inject export default module.exports; if it's found. I'll try taking a look at this when I get a chance.

[coreyfarrell]

Works for me, I just moved transform-commonjs to my common plugins list and verified it no longer breaks ES modules.

[coreyfarrell]

I've just found a case where this is failing to detect an ES module. I should have a test case soon.

[coreyfarrell]

We could also set isCJS = false if we find any imports/exports in the Program enter. To avoid the cases where you have something like:
import { require } from 'module'

This actually would not work since isCJS starts false and is set true if any 'CJS' features are detected. Maybe state.esmExports would be needed. I'm thinking that if ESM export keyword is found this should block setup of the module variable and export default module.exports;. In that case only require() statements should be processed to replace with import statements.

[tbranyen]

IMO if we find any clue this is an es module, we need to abort out, or make it a toggleable flag to handle requires.

[tbranyen]

I think the logic is "good enough" for now, and will continue to get better. I'm going to close this since there's nothing else actionable at the moment.