Add moduleType option to override module type on certain files.

[cspotcode]

Attempting an implementation of #1342
Closes #1342

Be sure @jayaddison gets credit for #1376 in the release notes.

Also refactors createResolverFunctions into its own file; should break this into 2x PRs later.

TODOs

• mark all exported ts-internals.ts as @internal
• check docs; add missing
• Make --showConfig moduleTypes log correct, relative paths same as include (not done, but extracted to Make --showConfig moduleTypes log correct, relative paths same as include #1389)
  • When include is specified in an extended tsconfig in another directory, --showConfig correctly updates relative paths so they render correctly.
• address missing coverage with test cases

[codecov]

Codecov Report

Merging #1371 (1ec6b05) into main (4e7fcb7) will decrease coverage by 2.74%.
The diff coverage is 68.03%.

Impacted Files	Coverage Δ
src/bin.ts	91.40% <ø> (ø)
src/ts-internals.ts	47.94% <46.82%> (-10.68%)	⬇️
src/resolver-functions.ts	84.09% <84.09%> (ø)
src/index.ts	78.70% <92.30%> (+0.27%)	⬆️
src/module-type-classifier.ts	92.30% <92.30%> (ø)
dist-raw/node-cjs-loader-utils.js	90.38% <100.00%> (+0.58%)	⬆️
src/configuration.ts	93.82% <100.00%> (+0.23%)	⬆️
src/esm.ts	87.50% <100.00%> (+2.39%)	⬆️

[jayaddison]

👋 thanks for the help in #1337 @cspotcode - glad to offer a bit of help in return (or to try to, at least :)). If you could provide some of that guidance re: test cases I'll start to take a look soon.

[cspotcode]

Since the coverage report is pretty big, I'll highlight some specific lines and the tests that will cover them. Please don't let the verbosity scare you; I erred on the side of including too much detail.

Missing coverage: https://app.codecov.io/gh/TypeStrong/ts-node/compare/1371/diff#D2L115

The test I wrote for this PR (https://github.com/TypeStrong/ts-node/pull/1371/files#diff-1d9efcccb250238d73480e5c49afae61494dd3103e9a16c38d36979f27c6ff90R1468-R1474) tests when you try to require() a script from within a CommonJS file. The overrides are checked to see if the target should follow the package.json or if it should be overridden to be CJS or ESM.

However, there's another code path I neglected to test: when you try to import something (not require) from within an ESM file using the --loader. In this situation, the overrides still need to be checked the same way as above. The missing coverage lines linked above indicate we are not testing this codepath. We need to be sure that overrides can force a file to be treated as CJS or as ESM. Either should load, but the CJS script will have typeof require === 'function' and the ESM script will have typeof require === 'undefined'.

In the linked code coverage, those 2x return statements are when there's an override forcing a file to be CJS and ESM, respectively.

I believe you should be able to copy-paste the test I wrote and then modify it to test this new situation. You can see I added a ./tests/module-types directory which is a mini-project, and then the tests are running ts-node in that directory and verifying that the output looks correct. In the mini-project, I've written scripts that will intentionally fail if the CJS/ESM overrides are not working correctly.

When our tests run, they actually npm install ts-node into the tests subdirectory, which makes them a close simulation of real usage. It also means you can play around by doing cd ./tests/module-types and then running commands such as ../node_modules/.bin/ts-node ... or node --loader ts-node/esm .... That should help to quickly create the test cases because you won't need to run ava to confirm it's working.

[jayaddison]

Getting closer to having the additional test coverage available to offer here.

One question so far: when using node --loader ts-node/esm ... as the command-under-test, is there a way to indicate the path to the tsconfig.json file, similar to the --project flag when using the ts-node command directly?

[cspotcode]

I think we use either the TS_NODE_PROJECT env var, or else we rely on auto-detection based on CWD. But I believe for these tests the cwd is one level too high so we use the env var. You might have some luck copying what we do for other --loader tests.

…

On Sun, Jun 20, 2021, 1:01 PM James Addison ***@***.***> wrote: Getting closer to having the additional test coverage available to offer here. One question so far: when using node --loader ts-node/esm ... as the command-under-test, is there a way to indicate the path to the tsconfig.json file, similar to the --project flag when using the ts-node command directly? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1371 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC35OEYGR2KZMIAIFCB53TTTYNFDANCNFSM46NNMPBQ> .

[jayaddison]

Thanks for the credit offer @cspotcode; only required if the tests end up being enabled in the test suite. I'll take a look into the volta Node version manager soon, but don't wait on me (and feel free to remove the commented tests (and credit) if they're not ready; less noise in the codebase).

[cspotcode]

No worries, I feel confident finishing this myself. After merging your PR, I spent some time tweaking and squashing errors. I have one more test failure to eliminate, but after that, I think we're good to go!

…

On Fri, Jul 9, 2021, 3:52 AM James Addison ***@***.***> wrote: Thanks for the credit offer @cspotcode <https://github.com/cspotcode>; only required if the tests end up being enabled in the test suite. I'll take a look into the volta Node version manager soon, but don't wait on me (and feel free to remove the commented tests (and credit) if they're not ready; less noise in the codebase). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1371 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC35OESBYOE65MLIZUYVYLTW2TFTANCNFSM46NNMPBQ> .

[cspotcode]

Only failure is a drop in code coverage, but I've reviewed the coverage report to determine that everything important is covered.