-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add resolved typeRoots
into the discarded includes to maintain global types.
#22
Add resolved typeRoots
into the discarded includes to maintain global types.
#22
Conversation
This is an idea to fix gustavopch#20. Instead of discarding all includes, which seems to discard required global `.d.ts` files, can we use the `typeRoots` a config may have setup instead? 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. - Often `typeRoots` will import a fair bit, this could still include a large % of your codebase. My `typeRoots` includes `node_modules/@types`–this may defeat the purpose gustavopch#18 set out to resolve.
@kylorhall As you said, it won't work for all use cases. Even if it's not a monorepo, some people (like me) use |
@kylorhall Also, I think
That's enough for TypeScript to read the |
Any named typeRoots folder(s) should work:
🤔 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:
Maybe this minimal repo will help clear things up: https://github.com/kylorhall/tsc-files-types-example Follow steps to replicate, a commit to |
@kylorhall Thanks for the repro! Now please try to move |
…est scenario. The resolution just appears we're using typeRoots incorrectly! See comment: gustavopch/tsc-files#22 (comment)
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:
{
"compilerOptions": {
- "types": ["jest", "node"],
"typeRoots": ["./types/", "./node_modules/@types"],
},
- "include": ["./src/", "./types/"],
+ "include": ["./src/"],
} With that, I wholeheartedly agree that doing Closing, will comment in the issue with a link back to here. For me on a single file:
|
@kylorhall Glad to know! Thanks for the detailed explanation. |
I'm locking so the discussion is centralized in #20. |
This is an idea to fix #20.
Instead of discarding all includes, which discards required, global
.d.ts
type files, can we use thetypeRoots
a config may have setup instead?For me, this basically results in:
I don't think this quite hits the mark, however:
typeRoots
in this manner? I do not know.resolveFromRoot
is not even required..typeRoots
will import a fair bit, this could still include a large % of your codebase. MytypeRoots
includesnode_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 theremainingArgsToForward
to be frank.