Ts-jest: Look into whether we can force commonJS modules always

[GeeWee]

We get a lot of questions about like #340 where the modules are explicitly set in the .tsconfig file. We should test out whether we can always force commonJS modules, no matter what they're set to in the config file, because otherwise jest will not work.

This might run into some issues with babel, that'll need to be investigated.

[GeeWee]

Currently it seems like babel works fine with CommonJS modules. I think we should force them, and then document it. It's only a problem if someone has a custom babel config that for some reason doesn't like CommonJS modules, and then I think it should be their responsibility to fix the issues.
Thoughts @kulshekhar @morajabi @bcruddy ?

[kulshekhar]

I'm not so sure about this. Taking away something that the users currently have seems like too drastic a step. Also, this might be a bit hasty given that node is likely to support ES modules.

Could a simple warning in the console not suffice? Like the one we're showing currently when __TS_CONFIG__ is used? A comment that is shown when module is not commonjs and shows the contents of the new config file for testing (extending from the main config file and overriding module to commonjs`) - for example

[GeeWee]

That's actually a really good idea.

[eteeselink]

If I may add a little to this discussion: Our tsconfig.json has module set to esnext, so that webpack resolves the imports and not TypeScript. This is the only way we've found to make dynamic imports with code splitting to work with TS and Webpack.

This means that we ran into this problem when adding our first TypeScript tests. Unfortunately, ts-jest only overrides module to "commonjs" when it is not given, not when it is given but set to something else.

It would be great if there was an option (maybe even one that defaults to true, although that may be a breaking change) which makes ts-jest always override this setting, even when it's already set to something else.

Currently, the only workaround I came up with that doesn't require us to manually keep a duplicate tsconfig file is this in our package.json's scripts section:

 "test": "sed \"s/esnext/commonjs/\" src/tsconfig.json > src/tsconfig.test.json && jest"

And then we point ts-jest to tsconfig.test.json (which is gitignored).

This works well enough, but it's hacky and messy.

[kulshekhar]

@eteeselink why not extends an existing tsconfig file and override the setting that's different?

[eteeselink]

Because I didn't know about extends :-D Thanks!

Hmm, maybe a lot of the practical problems with this could be solved by adding a recommendation to the README such as "make a tsconfig.jest.js that extends your real tsconfig and sets module to commonjs. IMO this might be a great general path towards fixing this issue (i.e. by not fixing it and letting extends to the heavy lifting).

I'll happily PR if you think that makes sense.

[kulshekhar]

@eteeselink that sounds great. A PR with this explanation would be welcome :)

[eteeselink]

OK, PR made!

Of course I'm not the maintainer but IMO this is good enough to close this issue. There's still something to say for automagically patching tsconfig options like suggested before, but that's a potential breaking change, and with extends everything is explicit and the user stays in control. Maybe that's not a bad thing :-)

[huafu]

I'm going to close this now, if there is any issue with that, please re-open!