Allow specifying "require" option via tsconfig

[cspotcode]

A follow-up to #921, moved into a separate PR per review comments. The diff for this will look much better after #921 is merged.

Allows specifying --require options via tsconfig.

Questions

• create() and register() will parse the option from tsconfig. Should they also do the require()s? Right now that's only done by bin

[coveralls]

Coverage increased (+1.7%) to 83.636% when pulling 6fb4b8f on cspotcode:ts-node-config-requires into 866bce6 on TypeStrong:master.

[coveralls]

Coverage decreased (-21.9%) to 62.105% when pulling e7690b9 on cspotcode:ts-node-config-requires into 2ba0a8f on TypeStrong:master.

[cspotcode]

I came across another use-case for this: using ts-node with tsconfig-paths via shebang.

#!/usr/bin/env ts-script
// Shebang loads ts-node options from local tsconfig.json
import * as localFooPackage from 'packages/foo';

// in tsconfig.json
{
  "ts-node": {
    "requires": ["tsconfig-paths/register"]
  },
  "compilerOptions": {
    "baseUrl": ".",
    "paths": {
      "packages/*": "./local-packages/*" // This example might not be perfect; I haven't used "paths" in a while
    }
  }
}

The alternative is to import tsconfig-paths/register at the top of every entry-point file. Which I'll grant isn't the worst thing in the world, but it's a little bit messy.

[ArkadyDR]

Hi @cspotcode,

I've found myself wanting this behaviour in one of my projects - for a similar reason to the use case you outlined above. I'd like to use tsconfig-paths with some paths defined in tsconfig.json, but want developers to be able to run some loose scripts (that may import some shared code using paths) via npx ts-node <script path> without needing to bolt on the -r tsconfig-paths/register. As you mentioned I could put the register at the top of each of the scripts, but given the tsconfig.json file specifies the paths it'd be nice if any execution using that project could load the requisite module for understanding the project (and perhaps any other bootstrapper code common to the project).

Given the age, are you likely to finish this PR? If not I might attempt something similar when I get some free time!

[cspotcode]

@ArkadyDR I've merged the latest master so this PR is ready for another review.

blakeembrey and I didn't agree whether or not this should be a feature, so I shelved it in order to get #921 released. I still think it's a good feature for the reasons that you cite, but we'll need to come to agreement.

@blakeembrey, want to revisit this?

[codecov]

Codecov Report

Merging #925 into master will increase coverage by 0.28%.
The diff coverage is 78.57%.

@@            Coverage Diff             @@
##           master     #925      +/-   ##
==========================================
+ Coverage   74.91%   75.20%   +0.28%     
==========================================
  Files           6        6              
  Lines         606      617      +11     
  Branches      141      146       +5     
==========================================
+ Hits          454      464      +10     
  Misses        100      100              
- Partials       52       53       +1     

Flag	Coverage Δ
#node_10	72.55% <78.57%> (+0.19%)	⬆️
#node_12_15	72.93% <78.57%> (+0.19%)	⬆️
#node_12_16	72.93% <78.57%> (+0.19%)	⬆️
#node_13	74.87% <78.57%> (+0.12%)	⬆️
#node_14	74.87% <78.57%> (+0.12%)	⬆️
#typescript_2_7	74.71% <78.57%> (+0.29%)	⬆️
#typescript_latest	73.90% <78.57%> (+0.30%)	⬆️
#typescript_next	73.74% <78.57%> (+0.31%)	⬆️
#ubuntu	74.87% <78.57%> (+0.29%)	⬆️
#windows	75.04% <78.57%> (+0.28%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/index.ts	77.45% <76.92%> (+0.21%)	⬆️
src/bin.ts	70.34% <100.00%> (-0.18%)	⬇️
src/esm.ts	68.88% <0.00%> (+2.22%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 866bce6...6fb4b8f. Read the comment docs.

[cspotcode]

CI seems to be stuck but as far as I can tell all tests are fixed. It properly resolves require() targets relative to the tsconfig file. And I renamed the option from requires to require to match the CLI flag.

[blakeembrey]

want to revisit this?

Absolutely, sounds like a good idea now that we shipped the basic feature.

[blakeembrey]

Should we merge these lists together? Seems odd to have the CLI entirely overwrite the options when specified.

[cspotcode]

Yeah, that makes sense. I can't think of a good use-case where you'd want the --require flag to suppress whatever was specified in the tsconfig.

[blakeembrey]

Thoughts on doing the require for these options within index.ts so configuration flows in a single direction? It might make it easier instead of needing to resolve and pass it back to the CLI which requires. Also, this would ensure we cover the programmatic usage of register() and via ts-node/register.

[cspotcode]

Yeah I like that.

The CLI uses Module._preloadModules; do you know if that function is doing anything special that will break if we switch to a normal require() call? There's nothing wrong with _preloadModules but I don't really understand what it does other than require() stuff.

[blakeembrey]

Yeah, I recall I tried to change it one time. Feel free to keep the CLI logic as it is and build the internal one separately for now so there's no regression. Here's the implementation in node.js: https://github.com/nodejs/node/blob/ec2ffd6b9d255e19818b6949d2f7dc7ac70faee9/lib/internal/modules/cjs/loader.js#L1400-L1417.

Here's where I had changed it at one point, but I can't seem to find the issue. I should have linked things better in the past 🤕 2e99c50#diff-ee45801c137c3b9f9d8ec7ece340a514R197.

[blakeembrey]

The main thing it's trying to ensure is that the require() is occurring from the place where you run the script, not where ts-node lives.

[cspotcode]

Ok, I don't think there's anything wrong with calling _preloadModules from within register(), so I'll keep doing that.

Should it happen in register() or in create()? I feel like register() makes more sense because that's where we "install" the environment. But source map support is installed in create().

[ArkadyDR]

@ArkadyDR I've merged the latest master so this PR is ready for another review.

Thanks @cspotcode and @blakeembrey for jumping on this so quickly!

Yeah, that makes sense. I can't think of a good use-case where you'd want the --require flag to suppress whatever was specified in the tsconfig.

If it helps, my expectation would be in line with where you're going - I would think that any CLI requires would be in addition to what is in the config, although you may want to de-duplicate them if there would be an overhead in processing something twice? If someone had a case where one of the configured requires was innapropriate for a script, the solution would probably be to spin off one or more extra tsconfig files (using the extension behaviour).

[cspotcode]

Ok, here's how the new behavior works:

create() will accept a require array in the programmatic options, merge it with the array it pulls from tsconfig.json, but will not require() anything.

register() will _preloadModules() the array it gets from create().

When create() merges the arrays, tsconfig items are first, then the programmatic items / those from the CLI --require. TBH I couldn't think of a good reason to prefer one ordering over the other.

Items pulled from tsconfig.json will be eagerly resolved at parse-time to absolute paths via createRequire(pathToTheTsConfigFile).resolve(item). It will be as if the tsconfig file had required them.

Programmatically passed items are not resolved and are passed verbatim to _preloadModules. This matches the current CLI behavior for --require. Docs point out that you can pre-resolve and pass us absolute paths for best results.

[cspotcode]

@ArkadyDR this is released in v9.0.0: https://github.com/TypeStrong/ts-node/releases/tag/v9.0.0