Add resolved typeRoots into the discarded includes to maintain global types.

[kylorhall]

This is an idea to fix #20.

Instead of discarding all includes, which discards required, global .d.ts type files, can we use the typeRoots a config may have setup instead?

For me, this basically results in:

-includes: [].
+includes: [
+  '/Users/…/repo/types',
+  '/Users/…/repo/node_modules/@types',
+],

I don't think this quite hits the mark, however:

• Does everyone use typeRoots in this manner? I do not know.
• Likely will not work for a monorepo with nested tsconfig as files are resolved to the root. Maybe resolveFromRoot is not even required..
• Often typeRoots will import a fair bit, this could still include a large % of your codebase. My typeRoots includes node_modules/@types.

Ultimately this may defeat the purpose #18 set out to resolve…but I guess this could be documented. Alternatively, I might suggest you could pass --typeRoots ./types/ or something via CLI, but I wasn't so comfortable implementing that with the remainingArgsToForward to be frank.

[gustavopch]

@kylorhall As you said, it won't work for all use cases. Even if it's not a monorepo, some people (like me) use @types folder instead of types.

[gustavopch]

@kylorhall Also, I think typeRoots shouldn't even need to be duplicated into include to work. I usually do this:

{
  "compilerOptions": {
    "typeRoots": ["@types", "node_modules/@types"]
  },
  "include": ["src"]
}

That's enough for TypeScript to read the @types folder when type-checking — no need to also add that folder to include. Doesn't that work for you?

[kylorhall]

Even if it won't work for all use-cases … @types

Any named typeRoots folder(s) should work: ./typings, ./@types, etc; just pulls from what is in compilerOptions.typeRoots.

---

That's enough for TypeScript to read the @types folder when type-checking — no need to also add that folder to include. Doesn't that work for you?

🤔 No, should it?

That's the core problem defined in #20, unless I/others in that issue, are missing something…I always thought this was a common tsconfig: include: ['./src', './types']. So then the change in #18 to files: ['file.ts'], include: [] does not work as global (local) types will be forever missing.

compilerOptions.typeRoots doesn't seem to inject those global types whatsoever for tsc at all as include: ['./types'] appears required. Maybe I/we are just doing something wrong?

---

Maybe this minimal repo will help clear things up: https://github.com/kylorhall/tsc-files-types-example

Follow steps to replicate, a commit to files.ts will fail with tsc-files@1.1.3. But, with a yarn link tsc-files, this PR passes on the above repo, so maybe a starting point to try out a few more scenarios?

[gustavopch]

@kylorhall Thanks for the repro! Now please try to move types/global.d.ts to types/global/index.d.ts and run npx tsc-files src/file.ts. It should understand the global types and pass. Then add some type error like const x: string = 1 and run it again. It should detect the type error and fail. Try it and tell me if it works. If it does, then this PR won't be needed. Instead, we'll just need to advise users on how to use typeRoots correctly.

[kylorhall]

Try it and tell me if it works.

Sorry it took a while. It definitely worked in that example repo: kylorhall/tsc-files-types-example#1, but it took a long time to figure out why it didn't happen in the full org codebase…

I believe I've gotten to the bottom of this and it's a combination of several misconceptions…was a bit more complex than the provided repo. Even reading issues like microsoft/TypeScript#22217 and solutions like https://stackoverflow.com/a/45887328 (which suggested this bad practice), I still don't have an answer, but I think I've got it. I'll have someone else on my team confirm, but it does appear to just be a bad TypeScript implementation and perhaps a TypeScript documentation issue.

Our issue was the combination of these three settings:

{
  "compilerOptions": {
   "types": ["jest", "node"],
    "typeRoots": ["./types/", "./node_modules/@types"],
  },
  "include": ["./src/", "./types/"],
}

My steps to resolve:

1. 🤦 Remove types entirely. This meant only jest and node were auto-included from types or node_modules/@types. I didn't even look at it, it's essentially the same as include, except for types.
2. Remove ./types from my include paths, so just include: ['./src'].
3. I rebuilt a few things in ./types because they were not definition files…but that's irrelevant to this.

{
  "compilerOptions": {
-   "types": ["jest", "node"],
    "typeRoots": ["./types/", "./node_modules/@types"],
  },
- "include": ["./src/", "./types/"],
+ "include": ["./src/"],
}

---

With that, I wholeheartedly agree that doing includes: compilerOptions.typeRoots is completely incorrect…it might work in a few nuanced scenarios, but then you'd be explicitly including node_modules/@types for most people, it defeats the purpose. Thanks for the direction!

---

Closing, will comment in the issue with a link back to here.

For me on a single file:

• 🎉 v1.1.3 takes roughly ~6s to do a git commit -am "…" – so thank you!
• 🐌 v1.1.2 takes roughly ~17.5s to do a git commit -am "…"

[gustavopch]

@kylorhall Glad to know! Thanks for the detailed explanation.

[gustavopch]

I'm locking so the discussion is centralized in #20.