Unused @types files installed by node modules pollutes and breaks the main project

[falsandtru]

Even when we don't use it in the main project. For instance, I'm using del package only in the build script file and it is .js file not .ts file.

https://github.com/falsandtru/pjax-api/blob/master/gulpfile.js

But it polluted and broke the main project:

https://travis-ci.org/falsandtru/pjax-api/jobs/525654172#L512

This is caused by sindresorhus/del@0361dcc.

You shouldn't automatically include unused/unimported @types files installed by node modules in the compile targets because they usually won't be used when they are not imported explicitly (regardless of direct or indirect). Or you should provide a way to exclude unexpected @types files by tsconfig.json to avoid this problem as a temporary workaround.

TypeScript Version: 3.4.0-dev.20190425

Search Terms:

Code

You can reproduce this problem also with https://github.com/falsandtru/spica.

1. clone https://github.com/falsandtru/spica
2. checkout v0.0.247.
3. run npm i
4. run npm i del@4.1.1
5. run gulp ts:dist

Expected behavior:

pass

Actual behavior:

TypeScript error: src/throttle.ts(7,5): Error TS2322: Type 'Timeout' is not assignable to type 'number'.
TypeScript error: src/throttle.ts(28,5): Error TS2322: Type 'Timeout' is not assignable to type 'number'.

Playground Link:

Related Issues:

[falsandtru]

Another problem caused by #8836.

[falsandtru]

Namely with client-side projects current TypeScript can't use npm packages bundling their own type definition files which depend on @types/node even when they are not compile targets (e.g. An npm package which is used only in build scripts written in JavaScript such as del).

[falsandtru]

@DanielRosenwasser What is your thought after you deleted your comment? Self-bundled type definition files are going to destroy TypeScript on npm. You have to address this serious design problem ASAP. You can take a temporary measure.

[weswigham]

FYI, If you wanna disable the automatic inclusion of definitions from the project's node_module folder (irrespective of if they're imported) you can just pass a --types argument. Even an empty list will disable the automatic inclusion behavior. Generally we're assuming most projects are assumed to only include a single environement's dependencies in the modules folder, so the auto-include behavior usually picks up globals without need for configuration, so is a better default for most.

[falsandtru]

I can't use that workaround because of depending some @types packages: https://github.com/falsandtru/spica/blob/v0.0.247/package.json#L45-L47. @DanielRosenwasser commented the same workaround but he deleted it. Probably he thought this is not a good workaround because of this problem.

Generally we're assuming most projects are assumed to only include a single environement's dependencies in the modules folder

So you shouldn't use the same folder for @types installed by node modules (especially only for build scripts). To exclude them from default compile targets you should use a different and separated folder.

[weswigham]

That's fine, just use types: ["mocha", "benchmark", "power-assert"]?

[falsandtru]

Of course I know it but such wasteful additional work will be a bad user experience. This problem must be resolved at upstream.

[falsandtru]

I propose that referring package.json like types: "package.json" and making it default.

[DanielRosenwasser]

@weswigham the problem is that the packages include the /// <reference path="types" />, which you can't prevent from loading. So even if you exclude node somehow, Node will always be picked up if it's present.

[falsandtru]

If you pick up @types packages from package.json, you don't refer other unnecessary packages. Thoughts?

[weswigham]

@weswigham the problem is that the packages include the /// , which you can't prevent from loading. So even if you exclude node somehow, Node will always be picked up if it's present

How is that a problem? If a file has a triple slash reference to other types, then it has a hard dependency on them, and needs them present...

[falsandtru]

The problem is separating or filtering, not installing.

[falsandtru]

Can you label this issue "bug" to inform this problem of package developers?

[weswigham]

@falsandtru If anything you should go report a bug on the type definitions that have a /// external reference they don't need.

If you pick up @types packages from package.json, you don't refer other unnecessary packages. Thoughts?

If we ever read a package.json file for configuration (eg, for project references) I'll keep it in mind (but most people list eg, @types/node as a dev dependency so even that is flawed); but right now we require all config be in a tsconfig ❤️

[falsandtru]

@falsandtru If anything you should go report a bug on the type definitions that have a /// external reference they don't need.

For instance, del package must not bundle type definitions referring @types/node yet. You should stop such bundling.

we require all config be in a tsconfig heart

Then you have to find another solution.

[weswigham]

For instance, del package must not bundle type definitions referring @types/node yet. You should stop such bundling.

I mean, if it needs the dependency it needs it, and it's up to you (the consumer) to split your environments up - be that via separate node_modules or via explicit --types options.

[falsandtru]

No, this problem appears with very common, correct, and typical usage. Resolving this problem is a responsibility of TypeScript. This is one of design-level problem based on #8836.

[falsandtru]

That is, to use TypeScript, do you really require developers to make another separated environment to use build scripts written in JavaScript? It is nonsense.

[weswigham]

That is, to use TypeScript, do you really require developers to make another separated environment to use build scripts written in JavaScript? It is nonsense.

Strictly we don't, but if you don't, there's a chance you need to pass an explicit --types option.

[falsandtru]

The problem is that chance will be indispensable sooner or later. You didn't have expected such an ending. You have a grace period but you would lose it if you don't address this problem.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[falsandtru]

At least this is not an external problem.

[weswigham]

Mmmmmm, it is an external problem. I'm going to reiterate for people who find this thread:

There is no bug in TS's behavior. When not overridden, TS will load all type definitions in your node_modules folder (like a default include path). When your node_modules folder contains dependencies for multiple hosts (eg, bundle target and build time) you should run tsc (or whatever tool you use that invokes tsc) with a --types option equal to the ambient declarations that host expects. That means for test, maybe --types mocha or --types jest, for a node server, maybe --types node, for a frontend, maybe --types jquery. In many cases a --types with no argument (an empty list) will even suffice, if you don't rely on any implicit globals. You can include this in your tsconfig.json for your build, eg:

{
  "compilerOptions": {
    "strict": true,
    "target": "es6",
    "module": "commonjs",
    "types": ["node"]
  }
}

TL;DR: Pass --types to the compiler if you have an issue relating to conflicting globals caused by your multiple kinds of dependencies installed into the same place.

[falsandtru]

When a developer requires no @types definition and depends on a package like del, he has to define "types": []. And when he add some @types definitions, he has to list the all of them. This is wasteful. You are requiring developers to pay this wasteful cost.

[falsandtru]

A key point is types parameter behaves obstructively. Developers are required to make some additional efforts to avoid that obstructive behavior. When a feature requires such efforts, it is regarded as a design failure.

[falsandtru]

Therefore, the current types parameter is a failed feature because of a failure of the ecosystem (#8836).

[weswigham]

And when he add some @types definitions, he has to list the all of them. This is wasteful. You are requiring developers to pay this wasteful cost.

You only need to list packages with globals that aren't explicitly imported, not every package. Things you explicitly import need not be explicitly included via types.

Developers are required to make some additional efforts to avoid that obstructive behavior. When a feature requires such efforts, it is regarded as a design failure.

The alternative is worse. (And we could only even consider it recently) - imagine if the behavior of types: [] was the default - now if your package depends on node or weback-env or react-native, you explicitly add a /// <types name="node"/>, since it's the only way to pull these globals in. Now the choice of which global environment your package requires is locked for all your consumers and can't be changed, and will potentially conflict with any environment they attempt to provide. This already happens when type authors don't know better (since having an open ended environment usually requires forward declarations of things like buffers), but changing the default makes it the defacto norm, which is bad.

Working out the dependency tree from a package.json wouldn't magically be any better, either - we'd still see that you depend on del, resolve del's dependency on @types/node and load it, and still see your local dependency on @types/react-native and load that, too - it's just a roundabout way of discovering the same thing as the contents of the node_modules folder (although admittedly would handle hoisting via lerna better, though that's not too common).

There is no change we can make here that qualifies as a "solution" where nobody has to write any config down, and we already have a solution that requires a minimal amount of config. We only ask that you provide the name(s) of the ambient definition packages which are not directly depended upon by references in your code, so that we may load only those specified and those your code actively references (via import or triple slash reference).

[falsandtru]

You only need to list packages with globals that aren't explicitly imported, not every package. Things you explicitly import need not be explicitly included via types.

Indeed, the cost is reduced. However, developers have to define globals or an empty list anyway.

The alternative is worse.

I currently have no suggestions. So I currently don't request any concrete solution. I'm just saying you have to resolve this problem.

There is no change we can make here that qualifies as a "solution" where nobody has to write any config down

If you can't find a solution that requires no additional efforts, I also give up.

I write down the original issue #18588 but you shouldn't label that issue with external. That is in discussion although it is strange to you. Can you stop to make arbitrary wrong decisions?

[jimisaacs]

I have specified types: [], and still my vscode project uses node_modules/@types/node, why?

[jimisaacs]

TypeScript version: 4.1.5

This is my tsconfig.json file:

{
	"$schema": "https://json.schemastore.org/tsconfig",
	"compilerOptions": {
		"allowJs": false,
		"allowSyntheticDefaultImports": true,
		"allowUnreachableCode": true,
		"assumeChangesOnlyAffectDirectDependencies": true,
		"baseUrl": ".",
		"esModuleInterop": true,
		"forceConsistentCasingInFileNames": true,
		"importHelpers": true,
		"importsNotUsedAsValues": "error",
		"incremental": true,
		"isolatedModules": true,
		"jsx": "preserve",
		"module": "ESNext",
		"moduleResolution": "node",
		"noEmit": true,
		"noImplicitAny": true,
		"noUncheckedIndexedAccess": true,
		"noUnusedLocals": true,
		"noUnusedParameters": true,
		"preserveSymlinks": true,
		"removeComments": true,
		"resolveJsonModule": true,
		"rootDir": "./src",
		"skipLibCheck": true,
		"sourceMap": false,
		"strict": true,
		"target": "ESNext",
		"typeRoots": [],
		"types": []
	},
	"include": ["./src/**/*.ts", "./src/**/*.tsx"],
	"exclude": ["./src/assets"]
}

Still it picks up @types/node

[DanielRosenwasser]

If you grab a TypeScript nightly or TypeScript 4.2 RC, we have an --explainFiles flag that might be able to help you here.

[jimisaacs]

@DanielRosenwasser I found out the issue, and I don't know if it's typescript's issue or my dependencies, it feels like typescript's though 🤷

So I found with that very extensive --explainFiles flag, that I had various transient dependencies that did this in their declaration types:

/// <reference types="node" />

When I removed one, and ran --explainFiles again, it would find another. So I removed that. I kept doing that until --explainFiles didn't find it anymore. At which point, my original issue of @types/nodes being able to be used anywhere in my project is resolved.

my particular package "offenders" are:

• @react-pdf/renderer: which is here https://github.com/diegomura/react-pdf/blob/master/index.d.ts#L1
• ts-invariant: which looks like they removed in their source here https://github.com/apollographql/invariant-packages/blob/main/packages/ts-invariant/src/invariant.ts#L1, but the dependency I'm depending on which is @apollo/client still uses an old version
• subscriptions-transport-ws: which also looks like they removed in their source here https://github.com/apollographql/subscriptions-transport-ws/blob/danielr/fix-start-dos/src/server.ts#L1, but the dependency I'm depending on which is @apollo/client still uses an old version

[jimisaacs]

@DanielRosenwasser do you agree that this sounds like a typescript issue? Does it deserve a new issue to be opened?