Don't include all @types/ automatically, only dependencies from package.json

[ivogabe]

The compiler currently picks all type declarations from node_modules/@types. This causes that when a dependency adds some @types package, this is automatically added to the compilation. However, this is not desired if the dependency is not used, for instance since it is only used in the build process. This could cause a lot of issues; imagine that a dependency of a dependency (etc.) adds an @types dependency.

Real world example: gulp-typescript is now using @types for its dependencies; @types/node is now a dependency of gulp-typescript. This caused that @types/node is now included in the projects of users of gulp-typescript, and TypeScript will automatically find this.

Currently users can either include all @types (default) or include a specific list (by setting "types": [ ... ]. My suggestion is something between those options:

Suggestion

Instead of picking all files of node_modules/@types, first check if a package.json file exist in the current directory or a parent directory. If so, read that file and include all @types that are registered as a dependency or devDependency. If the package.json file does not exist, fall back to the current behaviour.

If a project does include gulp-typescript for instance, the compiler will read the following line:

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

The compiler will automatically include this when run with the declaration option. So, types will only be added if they are really used either by the project itself, or by one of the dependencies (recursively).

I think that this would solve a lot of issues and make it easier for projects to migrate to @types. What do you guys think?

[blakeembrey]

I like the idea of being more explicit, but personally I'd go further and avoid tying it to package.json (which would be yet another surprising behaviour for people - e.g. when prototyping in my tmp/ts-test directory it might read upward to a tmp/package.json and include types I don't want). I'd love if the default was instead to include no globals and they all had to be explicitly listed, but maybe that's just me 😄

[aluanhaddad]

@blakeembrey Not it is not just you.
I think there is a conceptual question here of how to define what the extent of a project actually is. Is it package.json, tsconfig.json, cwd?

There are so many different interpretations with so many different answers. ES Modules are pretty silent on this so we can't look to them for guidance.
In some ways tsconfig files seem like a natural fit but they are purely a TypeScript convention so they don't help if you are depending on JavaScript projects via allowJs and with many other scenarios. Also they need to be interpreted by many different kinds of tools for different purposes. This relates to #3469 and there is a lot of discussion about the problems with this approach.

What about package.json?. It gets sticky when you consider the scope of tools like npm which are used to install packages but don't really define the scope of a project the way one might want them to. For example, we may use npm to install packages, but it is Node which resolves them, and Node doesn't care about package.json. So if you don't have a dependency on a package listed in your package.json but it happens to, incidentally, exist on disk (for example because of a bad merge resolution or neglecting to prune and install after checkout) Node and TypeScript (in mirroring Node's behavior) will happily resolve it.

Then you have RequireJS, other AMD loaders, and SystemJS. These all have a clearer, although arguably still vague, definition of what a project or package is, with explicitly declared dependencies. This is very different from how Node works as the latter treats the file system as a dependency source, which may make more sense on the server (but is still probably a bad idea), and then falls back to global installations based on environmental variables and so on and so forth.

This all leaves a lot to be desired. Something Allen Wirfs-Brock has talked about is the lack of any metadata for ES Modules, maybe something like that in a future specification could help here, but it is difficult to say.

[ivogabe]

@blakeembrey

I like the idea of being more explicit, but personally I'd go further and avoid tying it to package.json (which would be yet another surprising behaviour for people - e.g. when prototyping in my tmp/ts-test directory it might read upward to a tmp/package.json and include types I don't want)

TypeScript will currently include those types, since it searches for the first node_modules directory in a parent directory. So this surprising behaviour will already occur.

I'd love if the default was instead to include no globals and they all had to be explicitly listed, but maybe that's just me 😄

I agree about that, but I'm afraid that that is annoying to users. For instance when they want to use jQuery, they should install jQuery itself, install @types/jquery and set "types": ["jquery"] (assuming they use jQuery as a global variable). Anyway, you can already opt-in to that behaviour using "types": [] and list all used types between the brackets.

@aluanhaddad I also don't like to add another configuration file to TypeScript, but, package.json will already contain a list of the used @types packages. They are already listed as a dependency or a devDependency.

I think that you should make a difference between explicitly resolving packages (using import or <reference />) and implicitly resolving packages (how global @types packages are resolved). Your examples are only referring to importing packages that are not listed in the package.json file, but that are accidentally in the node_modules directory. This causes that something works, whereas it shouldn't work. For @types this is the other way around: by including some other package, your compilation may fail whereas it would compile if the package was not included.

@types packages can break a compilation; you could get 'duplicate identifier' errors. Node overrides the types for setTimeout and setInterval, which can also break code using these functions.

[aluanhaddad]

@ivogabe Indeed, I was going off on a tangent about how TypeScript and JavaScript lack a formal definition of projects, packages, and scopes. Regardless, I apologize for going off topic.

package.json is the closest thing to defining a project that we have and I think the your suggestion has a lot of merit, but I think transitive dependencies should be resolvable when you navigating the source of direct dependencies. That is there should be an implicit scope for module resolution across packages.

[blakeembrey]

TypeScript will currently include those types

I agree, which there's already dozens of issues about. I'm suggesting trying to avoid adding new surprising behaviour.

Anyway, you can already opt-in to that behaviour using "types": [] and list all used types between the brackets.

I agree again, but this issue is about the defaults and the open issues suggest the current default could use improving. Sure, every user could just do that, but it's not a valuable default in that case.

your compilation may fail whereas it would compile if the package was not included.

I'm not sure I follow this, it's really the same use-case. They are both results of implicit behaviour, but if it helps imagine that I have a parent tmp direct that contains both @types/mocha and @types/jasmine and does fail as a result. I'd love to remove implicit behaviour, as that's what I see as the source of confusion. The current explicit behaviours I don't think are a problem, only the implicit ones.

Edit: To be clearer, I also think your idea is already a better behaviour than the current one, I just want to propose taking it further. Using package.json as the wall isn't terribly consistent with how node resolves.

[reshadi]

I think this issue is not related to 'definition of project' and should not be resolved by looking at package.json, tsconfig.json, or current root dir.

A big part of using es6 module system is to simplify reusing code on client and server. That means you may have one package.json in root folder, a "lib" folder for shared code, a "client" folder for client code, and a "server" folder for server code.
In you build, you will then invoke tsc twice, each time to compile certain "main" file in client/server folders.

The tsc "discovers" the list of files that need to be included in the compilation by walking the import statements. It already has an algorithm for module resolution, which it uses to find the .ts or .d.ts files to be include. All we need here is to add the node_modules/@types/* to the list of directories to search for .d.ts files. In other words, first try to resolve the types in the node_modules/ and then try node_modules/@types with exact same algorithm.
This way, the right types will be loaded on demand and there is no need to make implicit assumptions.

[aluanhaddad]

Those directories are already implicitly in the list of directories that TypeScript enumerates.

It is an implicit assumption to resolve packages from there. That is NodeJS behavior, not JavaScript behavior and not ESModule behavior.

The point about inferring dependency locations by walking import statements breaks down whenever imports are neither relatives nor absolute. This is the case when importing any external dependency, say $ from "jquery". "jquery" is just an abstract identifier and ES Modules don't help us here except in that they allow for configuration mapping.

[reshadi]

What I'm suggesting is that loading of these types can follow --moduleResolution value. If module resolution is "node", then the spec defines (https://www.typescriptlang.org/docs/handbook/module-resolution.html) which folders and in what order are searched. I think if the search include the node_module/@types as well and load types files lazily, this particular problem goes away.

[aluanhaddad]

It already includes node_module/@types as of TypeScript 2.0.

The only available resolution algorithms are "node" and "classic" and the latter is problematic for many reasons.

Could you clarify what you mean by load types files lazily? I'm not sure I follow.

[aluanhaddad]

I would like the ability to disable or configure the recursive nature of the search. For example when I set traceResolution I can observe the language service searching node_modules and node_modules/@types directories that exist many directories above the baseUrl or rootDir(s) I have specified, all the way up to the root of the file system. If I'm not using npm this is pointless. Even if I am using it, it doesn't imply that I want to walk all the way up to the root.

[reshadi]

Could you clarify what you mean by load types files lazily? I'm not sure I follow.

What I mean is that compiler should not load all files in the @types folder. Instead, it should only load the content of node_modules/@types/X only if:

• it sees an "import * as x from 'X';" statement
• it cannot find corresponding .ts or .d.ts files in the node_modules/X folders (according to the search algorithm for "node" resolution mode.

In this way, only the types of the modules that are actually referenced in the code are used.

The example I provided in #11949 shows a simple case where the code itself is compiled with es5, but an independent test runner modules needs to be compiled (later) with es6 target. These are all part of the same project, but as of now, tsc compilation fails.

[ivogabe]

I like the suggestion of @blakeembrey, as it would make everything more predictable. It is more restrictive however. I'd like to hear what others think about it.

@aluanhaddad No problem, it does add some important context to this topic.

@reshadi What you are suggesting is already implemented for modules (packages that you can include/require). The problem here is: how should ambient/global types (libraries that are exposed as global variables) be loaded? If your code contains import * as foo from "foo", the compiler will know that the types for foo are required, but if the code contains process, how should it see that the types for NodeJS are required?

The current solution is to include everything in node_modules/@types. As you pointed out, that means that unused modules are included too. You can disable that in your project by setting "types": [].

My initial suggestion was to read package.json to find those files, @blakeembrey suggested to default to "types": [] (meaning that nothing is imported).

[reshadi]

@ivogabe, in any type of application, we can assume there is a main or top module that imports others. One can explicitly say "import '@types/node';" to get the proper types picked up. Same can be done for a browser application. In fact, in this case, you might say the --types flag is redundant since the application could have a simple file that imports all these @types and pass it to the compiler.

Imagine, you may also want to have a WinJs, Cordova, Chrome Only, .... type of application. In those cases as well, the "main" of the application can import proper types at first and then import the rest of the code.

In fact, we ran into this problem in our project. An external module had an issue with Google Closure. While waiting for upstream to fix it, we use a modified local copy, but for the types, simply use the following to mix local copy with existing types:

import './lib_name';
import '@types/lib_name';

setting "types": [] could be a work around, but requires the knowledge of all necessary types to be imported explicitly.

[reshadi]

@mhegazy @DanielRosenwasser @blakeembrey
Any feedback / final thoughts on this and related issues?

thanks.

[blakeembrey]

@reshadi You should know all the global types you need to import, that's why I like it being explicit. In what situation wouldn't you know? These are globals/environments, so they shouldn't be controlled by anyone else except you. Reading the package.json is reasonable, but less predictable as it means a demo with a package.json will act different to one without it (which could cause unnecessary back and forth in issues, etc, when someone happens to do --save vs without --save).

Using import explicitly is not possible, I'm not sure how that doesn't cause a runtime error for you as the require would fail.

[reshadi]

@blakeembrey we have (private) external modules that contain .ts files instead of compiled .js + .d.ts files. The package.json of these other external modules also contain @types. So, npm install will pull in other types that the "current project" is not aware of and come as a dependency.

The module resolution does assume that external modules may have .ts files. So, this kind of use case was already considered before. However, the way @types is working now, sometimes can cause issues (as listed here and in #11949).

As for explicit imports of types, we just use something like this:
import "@types/qunitjs";

This will be normally resolved and works ok.
Why do you think it should not be possible and fail?

[blakeembrey]

Most of your comment above seems irrelevant, you described this issue and how NPM works, not sure if I missed something in it. I'm aware of the issues, that's why I commented a while back with what I thought was the best solution.

[reshadi]

@blakeembrey sorry my bad! I just realized that we have a browsrify phase in our build that excludes the @types modules. That's why I had not seen the runtime errors.

I thought more about this, even if require @types was doable, still compiler would need a way to associate types loaded this way with the import of actual modules. So, my suggestion may not work and solve that.

I agree with you that tying type loading to package.json can still cause problem, specially in cases where client/server code is sitting in one project and have different dependencies captured in the package.json.
But as I said above, it is not always the case you know exactly what types should be imported, in cases where external modules are pure typescript code and not .js + .d.ts code.

So, somehow the compiler needs to lazy load the types when it sees modules that it does not know about.

[blakeembrey]

But as I said above, it is not always the case you know exactly what types should be imported, in cases where external modules are pure typescript code and not .js + .d.ts code.

This case should be covered already with external module usage, and if globals are required it would be covered using <reference type>.

[tsvetomir]

Just got bitten by this.

If a project dependency references @types/lodash your code automatically has access to the global types from it:

class Foo {
  private foo: Set<string>;
}

This compiles successfully and adds a reference to lodash in the output d.ts file:

/// <reference types="lodash" />
export declare class Foo {
    bar: Set<any>;
}

It should throw an error instead as I didn't reference @types/lodash in the project.

[SBD580]

+1 much needed - causing many collisions between libraries

[WorldMaker]

Took too many searches to find this issue, but the @types/lodash issue seems particularly indicative of the problem. Should node module resolution even allow global declarations at all? Or at least shouldn't they be sandboxed to shadow spaces? There seems to be a need in cases like lodash to "shadow" global declarations, but pulling in a dependency of my devDependency shouldn't pull in global declarations. The obvious counterpoint is @types/node itself, but that gets back into the debate if that should be better handled through some sort of --lib or --environment system (like #9466) external to node module resolution.

[tsvetomir]

Since the compiler knows about node_modules and @types it might very well parse the package.json file and determine which @types/* we want to reference from there.

[mhegazy]

Took too many searches to find this issue, but the @types/lodash issue seems particularly indicative of the problem. Should node module resolution even allow global declarations at all? Or at least shouldn't they be sandboxed to shadow spaces? There seems to be a need in cases like lodash to "shadow" global declarations, but pulling in a dependency of my devDependency shouldn't pull in global declarations. The obvious counterpoint is @types/node itself, but that gets back into the debate if that should be better handled through some sort of --lib or --environment system (like #9466) external to node module resolution.

Global side-effects are visible at runtime, so there is no reason why they should not be visible at design time.

The problem arises from declaration files that are authored as global where their .js counter parts do not do that. for that the fix is to change the module declaration.
In general unless this is node, electron-env , mocha-env, etc.. no global delectation should be used.

[mhegazy]

@mhegazy @DanielRosenwasser @blakeembrey
Any feedback / final thoughts on this and related issues?

@types are included automatically intentionally to simplify the use of the declaration files. Empty "types" property in tsconfig.json (or --types on commandline) is explicitly supported for users who want to opt out of this behavior; indeed this requires users to know what @types packages they use, but that seems like a simple issue to resolve, given that users have installed them, or listed them in their package.json at some point.

So we would like to stick with the current design for the time being.

[mhegazy]

Since the compiler knows about node_modules and @types it might very well parse the package.json file and determine which @types/* we want to reference from there.

true. but the assumption is most users would get the automatic inclusion of @types and no reason to worry about what types are included/excluded. for users who are interested in changing this, they can specify types explicitly in their tsconfig.json.
since adding/removing dependency is a rather rare event in the life-time of a project (compared to building for instance), we avoided additional complexity both from usage and implementation perspective.

[blakeembrey]

given that users have installed them, or listed them in their package.json at some point.
since adding/removing dependency is a rather rare event in the life-time of a project

I just wanted to clarify, in case it was missed, that the biggest issue comes from transitive dependencies and things you did not install. As a result of NPM flattening and TypeScript discovery, any dependency change at any level could break the build. For example, adding or updating a dependency on @types/node, a transitive dependency being changed from module to global (has happened with @types/lodash breaking builds), or just plain someone relying on something working because of the flattening effect of NPM (which happens often).

I know this has bitten myself and has, in the least, bitten enough users of my modules also that the modules I spend time publishing receive the blame instead of whatever the real problem is (see https://github.com/TypeStrong/typedoc/issues?utf8=%E2%9C%93&q=%40types%20dependencies%20 for how many times I've received a PR or issue to move @types into dev dependencies because something has broken their build by using TypeDoc).

[mhegazy]

I just wanted to clarify, in case it was missed, that the biggest issue comes from transitive dependencies and things you did not install. As a result of NPM flattening and TypeScript discovery, any dependency change at any level could break the build. For example, adding or updating a dependency on @types/node, a transitive dependency being changed from module to global (has happened with @types/lodash breaking builds), or just plain someone relying on something working because of the flattening effect of NPM (which happens often).

I personally have ran into this issue, and my local test script has empty --types to avoid running into it :); my recommendation if you have a project that has more than one tsconfig.json (or that your build does not use the @types packages in your node_modules for some reason) to specify types in your tsconfig.json.

Though it may be inconvenient for bigger projects, we chose to error on the side of simplifying the getting stated scenario.

[WorldMaker]

@mhegazy The problem is very much globals in type definitions. It's fine for Typescript to automatically read every @types/, if Typescript had a better way for "scoping" globals to avoid collisions and/or better dealing with lib.d.ts in a more piecemeal way (many uses of global are just "shadow fills" for lib.d.ts features).

Global side-effects are visible at runtime, so there is no reason why they should not be visible at design time.

In many of these cases, these are global issues that aren't visible at runtime because A) maybe it is a devDependency used only in the dev toolchain, or B) maybe a polyfill is in place or its a typing for a part of code that will never be called if a polyfill isn't in place, but a library wants to provide types for both sides.

Polyfills, as one common example, are cases where Typescript's typing system doesn't entirely allow for the variety of real world implementation.

DevDependencies and the concepts of multiple environments of code in a single node_modules is someplace else where Typescript currently falls flat.

I'd like to see Typescript either:

1. Be smarter about @types/ resolution and loading, because the dependency of my devDependency isn't my dependency
2. Better sandbox global declarations in @types/ modules
3. Provide a better way for @types/node, lib.d.ts and similar "environment" and polyfill typings, a better way to ignore/assume types are provided by an environment/polyfill, and entirely disallow global declaration in node_modules

This issue follows suggestion 1 above, and seems from this distance the easiest of the three possible solutions. Is 2 and/or 3 something better to throw weight behind?

[mhegazy]

if Typescript had a better way for "scoping" globals to avoid collisions and/or better dealing with lib.d.ts in a more piecemeal way (many uses of global are just "shadow fills" for lib.d.ts features).

We looked into this a while back, see #4913. we could not come up with a way that removes the "bad" globals, but keeps the "good" ones like node.d.ts, lib.d.ts, etc..
The bottom line is polluting the global scope is a bad thing to do, declaration files should not do unless they really have to. and if they have to, then the errors you get are valid, since two globals with the same name do conflict.

In many of these cases, these are global issues that aren't visible at runtime because A) maybe it is a devDependency used only in the dev toolchain, or B) maybe a polyfill is in place or its a typing for a part of code that will never be called if a polyfill isn't in place, but a library wants to provide types for both sides.

Either the dependency exports a global, then again it is fair to see it in the global scope; or it does not, and if so, then it needs to be fixed.
The language has affordancees to define modules, global, and umd modules. the dependency should leverage these to expose an accurate interface.

Be smarter about @types/ resolution and loading, because the dependency of my devDependency isn't my dependency

that is not true. they are, you get types from them. if your dependency does not expose any of types from its dependency it should not be added. at least that is what we do for publishing @types

Better sandbox global declarations in @types/ modules

see my earlier comment.

Provide a better way for @types/node, lib.d.ts and similar "environment" and polyfill typings, a better way to ignore/assume types are provided by an environment/polyfill, and entirely disallow global declaration in node_modules

yes. but there are others. mocha env, electron env, etc..

[WorldMaker]

The bottom line is polluting the global scope is a bad thing to do

None of the examples I've seen "pollute the global scope", they are trying to deal with "environment optional types" (where the type itself is optional to the rest of the definition, but wants to be supported if it exists in the current runtime environment) or hybrid environments or parallel environments.

The language has affordancees to define modules, global, and umd modules. the dependency should leverage these to expose an accurate interface.

I'm saying it doesn't have enough affordances to describe an accurate interface. That seems pretty clear here to me. There should be some sort of "softer globals" to better represent polyfills and complex environments and parallel environments and environment optional types. The libraries here that have been mentioned causing problems are all environmental issues that Typescript does not model well today. (Module augmentation gets us part of the way, except signatures have to exact match, can change between lib.d.ts versions, and that's exactly the kind of brittle that we're dealing with here.) Without further affordances for a better ambient environment model then we are left with needing more smarts in Typescript's "node module resolution", as this issue asks for, whether that is to be a better resolution for node modules and follow node resolution methods better and ignore globals entirely or something else. But this current state is a snake pit waiting to catch anyone of any skill level unsuspectingly.

that is not true. they are, you get types from them. if your dependency does not expose any of types from its dependency it should not be added. at least that is what we do for publishing @types

I don't know how you structure your projects, but nothing I save to my package.json devDependencies are types that I expect to import in my runtime project, they are used for compile-time pipelines like Gulp only. Maybe you are unfamiliar with how this sort of thing typically operates, but I don't export my build pipeline to consumers and I don't expect my build pipeline to run in the browser and I much more rarely write Typescript that runs in my build pipeline than in my runtime environment. (A lot of people don't even care to consider writing, say, a Gulpfile in Typescript and might never make use of the typing information from their devDependencies, much less the dependencies of their devDependencies, but on the flipside some people want those types, but only within their build pipeline itself.) My node_modules could be considered two different worlds that simply live side-by-side. I don't expect upgrading a devDependency and pulling in new versions of it to break to break my "runtime side" because at this point in the Node world there is a pretty good contract between the two "worlds".

It would be fantastic if Typescript made it easier to do the right thing with respect to the disparities between these "worlds" automatically, dependencies versus devDependencies (and their dependencies), and avoid "contagions" between them. That's why this issue is here. To some extent I don't care how it gets solved, but whatever the solution, Typescript doesn't have a great story for dealing with global type definitions in node_modules.

node_modules doesn't allow global JS at all, so why should Typescript accept global types from it? That doesn't fit the node model. Sure, there are plenty of environments currently distributed this way, but that doesn't mean it makes sense long term.

[aluanhaddad]

node_modules doesn't allow global JS at all, so why should Typescript accept global types from it? That doesn't fit the node model. Sure, there are plenty of environments currently distributed this way, but that doesn't mean it makes sense long term.

That's not quite correct. The npm registry has a fair number of global packages and, significantly, many packages that are dynamic with respect to the environment that they populate at runtime.

[ivogabe]

@mhegazy You didn't consider build-time dependencies, which will currently pollute the build. Gulp-typescript provides type definitions, and should also add @types/node as a dependency, but doing so would add node typings to all compilations done with gulp-typescript which could break applications targeted at web browsers for instance. It could even happen that a dependency of gulp-typescript adds @types/node as a dependency, which could break a lot of users. I don't see a way to prevent that currently, or do I mis something?