Adding "extends": support to `'ts-node'` loading from `tsconfig.json` #1292

seivan · 2021-04-07T03:46:29Z

So that one tsconfig extending another will merge the 'ts-node' options specified in both.

readTsConfigExtendForTsNodeRecursively(cwd: string, ts, rawConfig) will read as far as it can and merge with previous config.
Each extend has lower priority than previous, so your original "ts-node" values will not be overwritten by the next extend and so forth.

Based on comment #1007 (comment)

codecov · 2021-04-07T03:53:46Z

Codecov Report

Merging #1292 (a1b49fd) into main (bf47068) will decrease coverage by 0.00%.
The diff coverage is 83.33%.

❗ Current head a1b49fd differs from pull request most recent head 3340af3. Consider uploading reports for the commit 3340af3 to get more accurate results
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/configuration.ts | 93.05% <83.33%> (-3.38%) | ⬇️ |

…, so that one `tsconfig` extending another will merge the `'ts-node'` options specified in both. `readTsConfigExtendForTsNodeRecursively` will read as far back as it can while keeping the current stack around so it keeps merging. Higher stack gets priority.

cspotcode · 2021-04-07T16:54:56Z

Linking to related tickets: #4, #921

cspotcode

Made an initial review. This will also need tests.

src/configuration.ts

cspotcode · 2021-04-07T17:00:36Z

src/configuration.ts

+  const result =
+    extendFile === undefined
+      ? filterRecognizedTsConfigTsNodeOptions(config[configKey])
+      : [extendFile]


Nit: get rid of the filter().map() chaining. We prefer code like const, if(), for(const a of b), etc.

@cspotcode It's not for everyone 🐼. Once you have a look over the logic itself, I'll swap it out.

cspotcode · 2021-04-07T17:02:13Z

src/configuration.ts

+      : [extendFile]
+          .filter((x) => x && typeof x === 'string')
+          .map((x) => x as string)
+          .map((extended) => resolve(cwd, extended))


Does this work when ./bar/tsconfig.json refers to ./bar/tsconfig2.json by relative path? Because the path is not relative to cwd? Does it work for "extends": "@tsconfig/node14/tsconfig.json"? (https://github.com/tsconfig/bases)

Regarding relative paths...
Not sure if I want to say yes, I do use this myself and I my cwd wasn't relative, but then again I am doing other stuff that might impact it.
Good call, would do best to extract dirname/basename from current config in order to make relative paths work - but I gotta check first.
There is another approach and that is to let ts itself try to find the file, there is a function that for that last I checked.

Regarding @tsconfig/node14/tsconfig.json
Yeah no chance that would work.
I am not familiar with that, is that something you distribute or TS?
If it's the latter, then I'd reckon there wouldn't be a ts-node key there?

We'll need tests for this, and we'll need to support typescript's "extends" resolution behavior. @tsconfig/node14 was just an example; our users can choose to "extends" from anywhere that typescript supports, and we'll need to match that behavior. Ideally, we figure out how TypeScript is doing this logic and do the exact same, using as many TypeScript APIs as possible to avoid duplication.

https://www.typescriptlang.org/tsconfig#extends

Right, if TS exposes that, then I guess best bet is to use it, but to reimplement it is overkill.
In general, I think the simplest approach is just following the links.

In regards to your previous points about relative paths, this is an easy fix:

.map(path => isAbsolute(path) ? [path] : [currentConfigPath, path]) .map(paths => resolve(paths)

However I don't care much for extends: "@tsconfig/node14/tsconfig.json", on the premise that I just don't approve of this path they're going down on.

Using relative paths and not CWD now.

However I don't care much for extends: "@tsconfig/node14/tsconfig.json", on the premise that I just don't approve of this path they're going down on.

Unfortunately tsc already does this so we need to match. Otherwise it gets confusing and we need to document how our behavior deviates, and I'll be the one answering the issues that get files.

I certainly understand why you might choose not to use this feature on your projects, but for ts-node, we need to match TSC's behavior or else we're making things confusing.

seivan · 2021-04-09T02:07:42Z

@dandv

regarding extending tsconfig, might what you pointed out be the reason why transpileOnly didn't work for me if placed in the parent tsconfig.json?

Correctomundo. You can see that by running tsc -p tsconfig.json --showConfig on your "final" tsconfig. It should show you what your final config looks like once TS gets its hand on it.

What I am trying to do here is somewhat of a hack really, it's just recursively following the extends path and collects ts-node key value pairs until it can merge them together.

cspotcode · 2021-04-09T02:44:38Z

I looked at TypeScript's source code.

parseJsonConfigFileContent, which ts-node calls, is responsible for discovering, parsing, and merging extended tsconfigs.

It internally makes this chain of function calls:

parseJsonConfigFileContentWorker
- calls parseConfig
  - calls parseOwnConfigOfJson
    - calls getExtendsConfigPath:
      - resolves the "extends" string into a path to the extended config file.
  - calls getExtendedConfig passing the path to the resolved extended config
    - calls readJsonConfigFile
    - calls parseConfig (recursing)
  - merges options from the extended and extending configs

We need to mimic the resolution logic of getExtendsConfigPath, but unfortunately it's not exported. However, it's pretty short, so we could copy-paste it. It calls out to nodeModuleNameResolver, which is exported. (Technically calling an internal signature, but that's ok; we call a few internals as long as they're exposed)

parseJsonConfigFileContent also accepts a cache, a JS Map, to store parsed config files. We can use it to avoid double-parsing each tsconfig file.

seivan · 2021-04-09T02:53:37Z

@cspotcode Sounds a bit like what I mentioned earlier
Looking at it https://github.com/microsoft/TypeScript/blob/38da7c600c83e7b31193a62495239a0fe478cb67/src/compiler/moduleNameResolver.ts#L956-L957

It seems like a simplified version will just do the job fine - unless you want to go ahead and use all that.

.map(path => isAbsolute(path) ? [path] : [currentConfigPath, path])
.map(paths => resolve(paths)

seivan · 2021-04-09T02:58:28Z

@cspotcode

parseJsonConfigFileContent also accepts a cache, a JS Map, to store parsed config files. We can use it to avoid double-parsing each tsconfig file.

Is this a cache that they offer while traversing the extend or something you had in mind to store?
There is no double parsing when following the extend for ts-node so I am assuming you mean the using cached content of initial parse?

cspotcode · 2021-04-09T03:54:22Z

Is this a cache that they offer while traversing the extend or something you had in mind to store?

I'm looking at the extendedConfigCache argument to parseConfig and getExtendedConfig.

cspotcode · 2021-05-28T20:55:38Z

@seivan do you plan to come back to this, or should I close it?

seivan · 2021-05-31T09:55:09Z

@cspotcode I actually think your original idea of supporting a single env variable with all the options was better. I did this for my fork and thought it was enough but as I understand, you want the exact same inheritance support as TS has with its extend keyword. I am not sure I want to go down that rabbit hole for just this.

Edit: To clarify, I also feel uneasy about non ts-config stuff inside a tsconfig file, it seemed liked a good idea at start, but eventually it can get messy if everyone started doing that.

Feel free to close it. Sorry for the delay!

cspotcode · 2021-05-31T20:44:08Z

@seivan Understood, no worries.

seivan mentioned this pull request Apr 7, 2021

ESM support: soliciting feedback #1007

Open

4 tasks

seivan force-pushed the feature/read-extended-tsconfig branch from a1b49fd to eac5065 Compare April 7, 2021 03:52

seivan force-pushed the feature/read-extended-tsconfig branch from eac5065 to 8e771ff Compare April 7, 2021 03:54

cspotcode requested changes Apr 7, 2021

View reviewed changes

Resolve extended tsconfig path relative to current config path, not cwd

3340af3

cspotcode added this to the next milestone May 15, 2021

cspotcode mentioned this pull request May 22, 2021

ts-node field declared in extended tsconfig.json #1328

Closed

cspotcode closed this May 31, 2021

cspotcode mentioned this pull request May 31, 2021

Implement "extends" support for "ts-node" options in tsconfigs #1356

Merged

cspotcode modified the milestones: next, 10.x.x Jun 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding "extends": support to `'ts-node'` loading from `tsconfig.json` #1292

Adding "extends": support to `'ts-node'` loading from `tsconfig.json` #1292

seivan commented Apr 7, 2021

codecov bot commented Apr 7, 2021 •

edited

cspotcode commented Apr 7, 2021

cspotcode left a comment

cspotcode Apr 7, 2021

seivan Apr 7, 2021

cspotcode Apr 7, 2021

seivan Apr 7, 2021

cspotcode Apr 7, 2021

seivan Apr 9, 2021 •

edited

seivan Apr 9, 2021

cspotcode Apr 14, 2021

seivan commented Apr 9, 2021

cspotcode commented Apr 9, 2021

seivan commented Apr 9, 2021

seivan commented Apr 9, 2021

cspotcode commented Apr 9, 2021

cspotcode commented May 28, 2021

seivan commented May 31, 2021 •

edited

cspotcode commented May 31, 2021

Adding "extends": support to 'ts-node' loading from tsconfig.json #1292

Adding "extends": support to 'ts-node' loading from tsconfig.json #1292

Conversation

seivan commented Apr 7, 2021

codecov bot commented Apr 7, 2021 • edited

Codecov Report

cspotcode commented Apr 7, 2021

cspotcode left a comment

Choose a reason for hiding this comment

cspotcode Apr 7, 2021

Choose a reason for hiding this comment

seivan Apr 7, 2021

Choose a reason for hiding this comment

cspotcode Apr 7, 2021

Choose a reason for hiding this comment

seivan Apr 7, 2021

Choose a reason for hiding this comment

cspotcode Apr 7, 2021

Choose a reason for hiding this comment

seivan Apr 9, 2021 • edited

Choose a reason for hiding this comment

seivan Apr 9, 2021

Choose a reason for hiding this comment

cspotcode Apr 14, 2021

Choose a reason for hiding this comment

seivan commented Apr 9, 2021

cspotcode commented Apr 9, 2021

seivan commented Apr 9, 2021

seivan commented Apr 9, 2021

cspotcode commented Apr 9, 2021

cspotcode commented May 28, 2021

seivan commented May 31, 2021 • edited

cspotcode commented May 31, 2021

Adding "extends": support to `'ts-node'` loading from `tsconfig.json` #1292

Adding "extends": support to `'ts-node'` loading from `tsconfig.json` #1292

codecov bot commented Apr 7, 2021 •

edited

seivan Apr 9, 2021 •

edited

seivan commented May 31, 2021 •

edited