microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.03k stars 12.49k forks source link

Suggestion: add excludeTypeRoots to tsconfig #18588

Closed k8w closed 4 years ago

k8w commented 7 years ago

TypeScript Version: 2.4.0 / nightly (2.5.0-dev.201xxxxx) 2.5.2

It's a old issue that haven't resolve yet.

17242 #17042

Resubmit this for old ones are closed.

Case

  1. Used many declaration in node_modules/@types
  2. @types/node bring some compile error (it is a frontend project, but @types/node is a dependency of other package)
  3. So it need to disable @types/node, while other

Suggestion

  1. Support excludeTypeRoots in tsconfig.json.
  2. excludeTypeRoots have higher prioiry than typeRoots
mhegazy commented 7 years ago

So far we have not got this request for many users. with types specifying what to include and typeRoots to specify the location to look for types it seems possible to address most scenarios. adding yet another include/exclude pattern for types is not attractive.

k8w commented 7 years ago

This issue is mainly happened in those full-stack library. (Both support Node and browser).

Some of this library depends on @types/node, such as Protobuf.js.

And this would lead to some error. etc. return type of setTimeout is different when there is @types/node.

mhegazy commented 7 years ago

My recomendation here is to set types in your tsconfig.json all the time. I have personally found it to be more predictable this way while working on multi-project repos.

k8w commented 7 years ago

@mhegazy It can't resolve the problem, you just try this:

npm install @types/protobufjs

Code

let shouldBeNumber = setTimeout(() => { }, 0);
shouldBeNumber = 123;

If @types/node is used, this will get a compile error, otherwise it will compile successfully.

Setting types to ["protobufjs"] (withoutnode) is useless. Because node is a dependency of @types/protobufjs, that's why I say it need a way to exclude.

mhegazy commented 7 years ago

I miss understood the situation then. this seems like a definition issue. include/exclude starts the process. but once there is an import or /// <reference types=".." /> the compiler will suck in the additional definition files. that is the same way include / exclude work today for files as well.

The definition file should not have a dependency on node if it does not need it.

k8w commented 7 years ago

The definition file should not have a dependency on node if it does not need it.

But actually, there is already many frontend packages depend on @types/node, such as @types/react-dom @types/protobufjs and so on...

mhegazy commented 7 years ago

But actually, there is already many frontend packages depend on

These are definition bugs that need to be fixed i would say.

k8w commented 6 years ago

Actually many definition have bugs. It impossible to make every developer submit a PR and wait author to update them. So it is very necessary to have a way to disable those definition with bugs.

Actually many definition from JavaScript project would have this problem. Indeed so many... You can find many issue in DefinitelyTyped everyday. such as:

Of course I can write my own definition in typings, but the problem is sometimes it would be conflict with the original one. That's why I need a option to exclude them.

k8w commented 6 years ago

Still have this problem, anyone have good way to resolve ? So many library have this problem, even @types/react-dom

fqborges commented 6 years ago

Just to up this, it is giving me headaches.

Venryx commented 6 years ago

Have the same problem with react-dnd@3.0.2. It depends on dnd-core, which depends on @types/node, which globally declares the require function, which causes a duplicate require declaration error.

I've tried using the paths property to redirect references to @types/node to an empty file, but it appears to have no effect:

"paths": {
  "../node_modules/@types/node": ["Frame/NPMOverrides/node.d.ts"],
  "node_modules/@types/node": ["Frame/NPMOverrides/node.d.ts"],
  "@types/node": ["Frame/NPMOverrides/node.d.ts"],
  "node": ["Frame/NPMOverrides/node.d.ts"],
}

EDIT: Have worked around the issue for now by setting compilerOptions.skipLibCheck to true in my tsconfig.ts file. Not ideal, but at least the compile error doesn't show up now.

fqborges commented 6 years ago

It can be managed using the postinstall workaround mentioned on another thread. Recommend using shx package for multiplatform support.

"postinstall": "shx rm -rf node_modules/@types/node && echo 'workaround for libs importing @types/node on browser environment'"

EDIT: from: https://github.com/microsoft/TypeScript/issues/18588#issuecomment-487561397 a better workaround than running rm -rf on postinstall: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/21310#issuecomment-434329726

garrettmaring commented 6 years ago

This should be supported.

OliverJAsh commented 6 years ago

My recomendation here is to set types in your tsconfig.json all the time

This recommendation isn't ideal for my use case, which is: I have a base tsconfig.json that defines types, and I want to extend it (using extends).

If I specify types in the extension config, it will override the types from the extended config (perhaps it could merge?). This unfortunately means I'm left with no option but to duplicate the list in both TS configs, which is a bit of a maintenance burden.

wallzero commented 5 years ago

Unless I have overlooked something, this would also be handy in lerna projects. I have some packages that don't require fancy libs and some that do. However, since the dependencies are hoisted, typescript scans the definitions from all the packages and determines that even the simple ones need esnext.asynciterable for example.

I can either prevent certain @types packages from being hoisted, or I can explicitly list the types in the simple packages tsconfig.json - but a blacklist option would be preferred.

AndrewJDR commented 5 years ago

It can be managed using the postinstall workaround mentioned on another thread. Recommend using shx package for multiplatform support.

"postinstall": "shx rm -rf node_modules/@types/node && echo 'workaround for libs importing @types/node on browser environment'"

You know something is wonky when a rimraf is needed as a workaround...

It seems like the powers that be on the typescript side think that this is a matter for package maintainers to deal with, but the reality is messier -- there are different packages that are alternatives for other ones (like react vs. preact for example) where people are trying to use these in the same project, and the solution for dealing with types is not always straightforward: https://github.com/developit/preact/issues/1036

None of the solutions proposed, such as defining types, or remapping using baseUrl/paths actually work. The rimraf is literally the only thing I've found that does work. Do we really want rm rf commands sitting around in our configuration files, waiting for a misplaced asterisk or slash to come along?

Please please please implement this.

lostpebble commented 5 years ago

By far this is one of the biggest annoyances in Typescript. Expecting that every single dependency's types are absolutely perfectly built before being able to build your code correctly - we all know this is impossible, developers are not perfect, configuration for TypeScript libraries can be difficult, and I'm struggling with this very thing with Google engineers who have completely messed up their types for libraries I'm using.

The fact that I can't easily just tell Typescript to ignore those types for now and spit out my code is so infuriating. At the core of, its still JavaScript - and these libraries have been built already - I can make use of them now, but TypeScript is being a real productivity blocker here.

These are definition bugs that need to be fixed i would say.

Is the current expectation to really go and fix every single library you depend on's types? What do you do while you wait for your pull requests to be accepted? There are certain situations where "make a pull request" is a viable answer - this is definitely not one of them - because its actually completely unrelated to the issue at hand.

sam-s4s commented 5 years ago

Yeah, I'm now facing this problem too. I'd love an exclude for types.

Unfortunately a recent change to angular-cli has added a type dependency in for "node", which breaks all my references to settimeout and setinterval (as their type is supposed to be number, and now is NodeJS.Timeout) :(

leethree commented 5 years ago

I'm seeing the same problem in my project and defining types in tsconfig doesn't seem to help. I have to file issues to library that includes @types/node to remove them. e.g. https://github.com/TooTallNate/node-data-uri-to-buffer/issues/8

Would definitely be helpful if there's way to just exclude @types/node.

pvinis commented 5 years ago

Having typeRoots keep the order for overloads would be also helpful. This way we can overload the buggy ones at least.

StephenHaney commented 5 years ago

Since this is tagged "awaiting more feedback":

We're in the same boat. Web libs with dependencies on server typings are polluting our types with incorrect node types.

The end result is @types/node is ending up in a ton of browser targeting projects.

RyanCavanaugh commented 5 years ago

Are people asking for excludeTypeRoots or excludeTypes ? It sounds like a lot more the latter than the former?

sam-s4s commented 5 years ago

I can only speak for myself, but I am after an excludeTypes :) (I agree, it looks like most people seem to want that)

ie. many of us need to exclude @types/node

RyanCavanaugh commented 5 years ago

Is skipLibCheck a dominating strategy for people in this situation? Or is the inclusion of node itself the problem?

sam-s4s commented 5 years ago

I haven't used that before, but it sounds like it would prevent ALL types from all node_modules packages, as opposed to just @types ?

(ie. you couldn't use Angular, which has .d.ts files for all its libraries)

-EDIT- (or does it just prevent errors inside node_modules packages that would kill the compile? this is not my problem - my problem is that node types are overriding browser types, like setTimeout should return a number, but with node, returns NodeJS.Timer)

StephenHaney commented 5 years ago

@RyanCavanaugh I believe in our case it would be excludeTypes. Here are two situations we've hit recently, and I think it summarizes what most people are seeing:

  1. Create React App adds @types/node and Styled Components adds @types/react-native... these two collide so that a fresh CRA install w/ Styled Components is unable to build (if skipLibCheck is off). Ref: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/33015

  2. Many browser-targeting libraries add @types/node. react-dom is a visible example, though recently fixed. Some common DOM window functions like setTimeout infer to the incorrect node typing for setTimeout, which creates a TS compilation error – unless you write window.setTimeout. All CRA apps have this issue as far as I know - any library that works across node and the browser, but targets the browser, will likely have this. Ref: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/21310

There may be existing config solutions to these problems. I am far from an expert in this area - just summarizing what I've found.

There may be better proposals to fix these: allowing the config to specify the precedence of types to put DOM types over node types would solve bullet 2 most accurately for CRA apps. Further, it might be the responsibility of the type definitions to be more correct in the first place – but it's happening in multiple libraries, so it seems the maintainers need some help. There's not one huge issue, but I've found a long tail of issue threads running into this same core difficulty of unwanted types from dependencies.

bard commented 5 years ago

For those using yarn, see point 1) of https://github.com/DefinitelyTyped/DefinitelyTyped/issues/21310#issuecomment-434329726 for a better workaround than running rm -rf on postinstall.

alamothe commented 5 years ago

@types/node are especially problematic. It's difficult to avoid this package being included in a front-end project (React or React Native).

@types/node has many global definitions (console, setTimeout etc.) which are not compatible with front-end definitions in @types/react-native.

From my experience, TypeScript will choose one set of definitions or the other at random. So it's a coin toss whether the project compiles or not.

Consider @types/node:

declare function setTimeout(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timeout;
declare namespace setTimeout {
    function __promisify__(ms: number): Promise<void>;
    function __promisify__<T>(ms: number, value: T): Promise<T>;
}
declare function clearTimeout(timeoutId: NodeJS.Timeout): void;
declare function setInterval(callback: (...args: any[]) => void, ms: number, ...args: any[]): NodeJS.Timeout;
declare function clearInterval(intervalId: NodeJS.Timeout): void;
declare function setImmediate(callback: (...args: any[]) => void, ...args: any[]): NodeJS.Immediate;
declare namespace setImmediate {
    function __promisify__(): Promise<void>;
    function __promisify__<T>(value: T): Promise<T>;
}
declare function clearImmediate(immediateId: NodeJS.Immediate): void;

While @types/react-native:

declare function clearInterval(handle: number): void;
declare function clearTimeout(handle: number): void;
declare function setInterval(handler: (...args: any[]) => void, timeout: number): number;
declare function setInterval(handler: any, timeout?: any, ...args: any[]): number;
declare function setTimeout(handler: (...args: any[]) => void, timeout: number): number;
declare function setTimeout(handler: any, timeout?: any, ...args: any[]): number;
declare function clearImmediate(handle: number): void;
declare function setImmediate(handler: (...args: any[]) => void): number;

Both are global definitions.

Not sure what's the solution... So far I used patch-package to comment out one set of global definitions.

falsandtru commented 5 years ago

I think TypeScript can automatically resolve this problem in many cases by checking _requiredBy field of package.json of installed @types packages. When the compiler has known, as a result of this checking, that an @types package is not a dependency listed in package.json (note that we can know it without seeing package.json file of the main project), this package should be excluded from the targets of auto loading. Can you adopt this logic?

amaschas commented 5 years ago

I believe I'm running into a similar issue with conflicting type definitions in downstream dependencies. In my case, koa-bodyparser and koa-joi-router both augment the koa module definition with a body parameter, but one defines the parameter as required and one does not. I've submitted a PR to DefinitelyTyped, but it would be great if there was a feature that allowed me to resolve these kinds of typing conflicts. Problems like this contribute to my impression that using Typescript results in spending most of my time solving typing issues rather than writing code.

toddwong commented 5 years ago

I'd suggest the .gitignore / .npmignore way, allowing mixing includes/excludes in one list, and priority them by order.

toddwong commented 5 years ago

I don't think it is a good idea, but is there chance to distinguish function overloads by return type?

falsandtru commented 5 years ago

As I mentioned at #31148, unused @types files installed by node modules pollutes and breaks the main project. It is caused by del package at that time. Now this problem came to be spread by another major package npm-check-updates. I warn the destruction of ecosystem is expanding. TypeScript must resolve the problem quickly. See a solution https://github.com/microsoft/TypeScript/issues/18588#issuecomment-492616933.

danielweck commented 5 years ago

When a package has erroneous TypeScript type definitions, I use the following workaround to completely disable the package's own *.d.ts:

noop.d.ts:

declare module "PACKAGE_NAME";

...and:

tsconfig.json:

    "paths": {
      "PACKAGE_NAME": [
        "noop.d.ts"
      ]
    },
dbartholomae commented 5 years ago

Same problem here, using the skipLibCheck workaround :-/

anilanar commented 4 years ago

Some stuff I've compiled about this problem when I was mad at TS again:


1) some common 3rd party type definition problems (applies to both typeRoots and packages that declare their own types):


2) typeRoot problems:


3) types from a pkg's package.json. These problems are mostly related to OS packages that have lackluster typings declared in their package.json.


4) versioning:


5) Disabling typeRoots and doing manual type whitelisting via typessolves only 1 problem: Avoiding unwanted @types packages.

MicahZoltu commented 4 years ago

This issue has been open for over 2 years now, has no one gone and submitted a PR to React to fix their bad types in all that time? That certainly feels like the right solution to that specific problem.

That being said, I do think this issue deserves a solution for other reasons:

{
  "devDependencies": {
    "@types/node": "...",
    "ts-node": "...",
    "typescript": "...",
}
// source/index.ts
import * from 'path' // should fail, this library is meant to be used in node & browser!
// scripts/build.ts
import * from 'path' // should succeed, this is a build script that executes in nodejs

I have separate tsconfig.json files for the library and the build scripts, but in order to utilize types I need to include every type in the library, which is basically just duplicating the contents of my dependencies... all so I can ignore a single @types/node.

What I would really like is for node to be a first class citizen like dom and es2018 so I can simply not include it in my library tsconfig.json and include it in my build script tsconfig.json.

The core of the issue is that I have one package.json and multiple tsconfig.json files. The TypeScript projects all share a single node_modules folder so I don't have to download everything twice.

alloy commented 4 years ago

While the OP of this issue mentions “typeRoots”, I think most people (perhaps even OP?) end up thinking of the types property of compilerOptions. In any case, that is what I will be discussing; apologies if this in any way detracts from the OP. With that out of the way…

Having maintained the react-native typings for a few years now, I’ve seen various issues raised that all boil down to the same thing. Either people have a react-native project with a dependency that pulls in @types/node or they have a react dom project with a dependency that pulls in @types/react-native. I can’t find a currently valid example of the former, because people have since spent time on making the globals of @types/react-native and @types/node mergeable, but here’s one of the latter: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/33015

All suggested solutions come down to not letting tsc use the @types/* package that’s irrelevant to the env the user is targeting. What ends up being the problem to most users is the user-experience of doing so; which is to allow-list all the types the user does want, by specifying them using the types property of the compilerOptions. Rather, people would like to deny-list the few they don’t want. (Currently people end up doing this by e.g. adding those packages to a .yarnclean file, which automatically removes matching entries after installing packages.)

Now, I’m not necessarily a proponent of having more options–in general–however, precedent has been set by also having include and exclude options for files. It feels natural then for this pattern to extend to other options that have at least 1 way to be explicit, besides a greedy implicit default. I.e. currently there’s 2 types options:

  1. Include all packages found in node_modules/@types by default
  2. Include only the ones explicitly specified with the types property of compilerOptions

I propose to add a third one:

  1. Include all packages found in node_modules/@types, but exclude those explicitly specified with a excludeTypes property

I have created a repo that both demonstrates the problem and comes with a simple tsc patch to add a excludeTypes property to compilerOptions: https://github.com/alloy/tsc-exclude-types-repro

cc @orta who I’ve been pairing with

orta commented 4 years ago

( and the base of a PR + test verifying it works )

MicahZoltu commented 4 years ago

@alloy Can you summarize here why it is not possible to fix the typings you linked? Most of the issues I see in this thread are just people who are trying to hack around broken type definitions, but IMO the real solution for those is to fix the type definitions rather than hack around their brokenness. If there are situations that are fundamentally unfixable, that would be good to describe in this issue.

lostpebble commented 4 years ago

@MicahZoltu it boils down to being productive. I said it earlier in this issue. Yes, it would be great if every single library had its types in order at all times- but it is abundantly clear that this is a pipe dream.

There have been countless hours wasted for me personally on this, when someone deploys a new version or adds types to their previously untyped library and they completely mess things up in one way or another, be it their types, or them including other types which are inadvertently not compatible with something else in my stack.

The core of this issue is productivity and not wasting another second on something so ridiculous. The fact of the matter is, the code does work, at the end of the day its just JavaScript, but TypeScript has come in and decided that if any types are slightly wrong, you just won't be able to compile today. Because they said so. Because they want to be overly strict, when they could just give us the option to say, for this specific library - just treat it as any!

Yes, libraries have issues and they will get fixed. I will still go and open up an issue about the types and try and help fix those issues - but flip, when I just need to push a new feature out the door and TypeScript gives me this crap, it really drives me up the wall, as I'm sure it does everyone else in this thread and the countless others who suffer silently.

Its frankly ridiculous to argue "Just fix the types..." at this point. We have reiterated the problem clearly now, over and over again. Please be more developer (and my sanity) friendly and see the problem here please.

alloy commented 4 years ago

@MicahZoltu Good point 👍

Theoretically it is likely possible to fix these issues, but it requires people to:

All of these rely on multiple people fully understanding the problem and checking all possible permutations of how people might load these dependencies. In short, it’s a very human task and thus inevitably will fail [again].

Because there’s no great builtin escape hatch for end-users to control what typings do not get loaded, this ends up being a maintenance burden for DT maintainers [such as myself] and results in frustrated [TS] end-users.

danielweck commented 4 years ago

my solution / workaround to “nullify” incorrect imported typings is to add a path definition to tsconfig.json such as:

"paths": {
  "offending-package": [
    "typings/noop.d.ts"
  ],

...with noop.d.ts containing:

declare module “offending-package”;

MicahZoltu commented 4 years ago

To be clear, I am an advocate of this feature, but for the reasons specified in https://github.com/microsoft/TypeScript/issues/18588#issuecomment-598037818 and I think that the fact that it will enable developers to drop bad transitive dependencies is a minor loss for the community.

The downside of enabling developers to drop transitive type dependencies is that it will give people an "easy path" to solving their "right now" problems that doesn't help the ecosystem move forward in a healthy way. While some people will take the time to submit a PR to the incorrectly typed library despite there being an easy workaround, the vast majority of people will just apply the workaround and move on. Some subset of that vast majority would have submitted a PR if that were the only option, while another subset would have gone and used a different library (one with proper types).

My fear is that by giving developers an easy path to "fix my immediate problem" that doesn't align with "fix the problem for everyone going forward" we likely will just end up with more broken code.

alloy commented 4 years ago

@MicahZoltu I hear you, but I don’t think ‘forcing’ people realistically leads to a culture of more fruitful collaboration. I think a pragmatic approach of having to add an entry to a denylist means we can have both:

lostpebble commented 4 years ago

The downside of enabling developers to drop transitive type dependencies is that it will give people an "easy path" to solving their "right now" problems that doesn't help the ecosystem move forward in a healthy way.

I kinda disagree. The ecosystem will still move forward. People use TypeScript because they enjoy the types and autocomplete that comes when using other people's libraries. If they need to switch that off for the sake of productivity in a single moment, it does not mean that they will ignore the fact that this library isn't typed anymore- especially if its something they use often. Have you seen how many issues pop up on regular JavaScript libraries asking people to add types / rewrite in TypeScript? Its clear that people do care, even when types don't exist from the get go.

I don't believe people will stop caring about their types being correct simply because they have been given the choice to be more productive by temporarily turning those types off. If that were the case, then TypeScript wouldn't have the adoption it does today. And honestly if they do never turn them back on, so what? Its their choice to code without types then. I'm certain that many people will still want their types back though.

And as @alloy said, "forcing" people simply because TypeScript sees this as a way for users to keep the ecosystem healthy isn't a good call either. It feels kinda sadistic and is wasting a lot of time for people.

tmtron commented 4 years ago

excluding types would really be great

As a workaround to disallow some globals (e.g. nodeJs process) in shared code, we use es lint no-restricted-globals

We also used this rule no-restricted-modules - but unfortunately it is now deprecated. And I don't see how this can be replaced by eslint-plugin-node

phiresky commented 4 years ago

Since I've not seen it mentioned, here's my workaround:

Create a package.json file containing just {"types": "index.d.ts"} in a subdirectory like typings/fake-empty/package.json and a empty index.d.ts file.

then in your main package.json, specify a dependency like "@types/node": "file:./typings/fake-empty"

This way, npm will install that empty version of that dependency in your root node_modules, which means it won't be able to hoist the real @types/node from your dependency. This means that the real @types/node will be in node_modules/react-native/node_modules/@types/node, and not affect your main project.

Same works for other types packages you want to prevent hoisting for.

This works better than the paths: workaround since that apparently does not work for global declares such as NodeJs.Timer etc.

alamothe commented 4 years ago

I have this problem again with yarn workspaces. My workspace contains projects some of which use DOM library and some don't. Some dependencies will use DOM library as well.

Now, the problem is that projects which don't use DOM library no longer compile, because TypeScript will include everything under @types even though it's not referenced by the project in any way.

The only solution seems to be to add DOM library even for backend projects, which kinda sucks.

EDIT: So clearly my ask is for excludeTypes, to be able to exclude a types package which is not referenced by the project or any of its dependencies.

weichensw commented 4 years ago

Two years after the original issue was submitted we still observe many definitelyTyped definitions have reference to @types/node and creating conflicts.

Just to name a few: @types/uuid @types/body-parser @types/cheerio @types/glob @types/jszip ... and many many others. and any packages that inherently depend on those

I think it is fair to say that "fixing the types" to resolve this problem is not easy and will take long time. Meanwhile, it will be really helpful if we could either control what type we to hoist, or make node part of the embedded types like DOM.