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

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?

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 9
  • Comments: 33 (20 by maintainers)

Commits related to this issue

Most upvoted comments

@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?

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 😄

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=✓&q=%40types dependencies 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).

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.

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.

+1 much needed - causing many collisions between libraries

image

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 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.

@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.

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).