Add .cjs as list of potential JS files

[crutchcorn]

When using type="module", there's still the ability to use commonjs style require and module.exports in .cjs files (as opposed to .js files).

This PR adds it as a candidate for files that can be handled using Liftoff

I've tested this within our close-to-release plopjs@3 release, which will support ESM

[phated]

I think this makes sense. However, in #68, we identified that falling back to node's loader will load the cjs file anyway. Can you explain a little more how plopjs loads things so I can understand why falling back doesn't work?

[crutchcorn]

Whoops! I missed #68, my bad.

For context, I'm using an await import here:

https://github.com/plopjs/node-plop/blob/master/src/node-plop.js#L198

Where plop then passes the configfile directly from Liftoff

https://github.com/plopjs/plop/blob/master/src/plop.js#L18-L24

https://github.com/plopjs/plop/blob/master/src/plop.js#L56-L59

However, when I was working on CLI E2E tests, I ran into issues with CJS files loading properly

https://github.com/plopjs/plop/blob/master/tests/esm.spec.js#L30-L39

This test currently passes, but if I remove the cjs config extension from here:

https://github.com/plopjs/plop/blob/master/src/plop.js#L22

This test then fails

[phated]

Well, I'm convinced! I'm happy to have a real-world use case to prove that it is needed. I'll get this merged and released today.

[phated]

@crutchcorn I'm trying to figure out how to write a test for this right now.

[crutchcorn]

@phated gotcha! I can see about adding them if that'd be helpful

Far from any rush though, we've already published v3 and it's working rather well with the extension changes :)

[phated]

That'd be a ton of help! I've not done much with "type": "module" stuff. It seems the issue is that the .cjs extension doesn't exist in the commonjs context, so we might need to stub it the same way we stub mjs.

[phated]

@crutchcorn do you still have some availability to get tests written for this?

[crutchcorn]

I took a quick stab and I'll admit that I don't understand the testing methodologies at play well enough to move forward.

Admittedly, the tests should look similar to mjs I think. The gist is basically:

1) CJS can be imported from anything (require in non-ESM files, import in ESM), regardless of type field values in package.json 2) CJS can use module.export (and other CJS) in type="module" projects 3) CJS should throw an error when using ESM exports and such, regardless of type field values in package.json

[phated]

@crutchcorn the issue, if I remember, is that our test harness is running under commonjs, so we can never test the cjs extension. I think that you'll need to exec some script in a different directory that enables the module context

[phated]

@crutchcorn I rebased this and added some tests. Can you sanity check them for me to make sure this is what you expect?

[phated]

Tests are passing. Just waiting on @crutchcorn's feedback 😄

[crutchcorn]

Tests LGTM!

[phated]

Thanks for getting this started and checking out the tests! Sorry it has taken so long to get released (it'll ship with the 3.0 changes)

[FDiskas]

Left over? https://github.com/plopjs/plop/blob/4433a201d9709e69c4398a88b3886d1fc593d6bf/packages/plop/src/plop.js#L21