Node split -- broke my typescript projects

[fearthecowboy]

Regarding PR #32567

// cc @rbuckton @RyanCavanaugh, @DanielRosenwasser

• I tried using the @types/node (10.12.21) package and had problems.
• I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript

I don't know what this split was intended to do, but I had some typescript compiles fail in my CI this morning.

The last build was using @types/node 10.12.19 (pre-split) and the build that failed for us was using @types/node 10.12.21. (package.json files were using ^10.11.4 and no package-lock.json was present)

When a library was built with 10.12.19, the .d.ts file for the library looked like this:

/// <reference types="node" />
import { Exception } from '@microsoft.azure/tasks';
import * as fs from 'fs';
// ... etc ... 

and when it was build with 10.12.21 it was missing the /// <reference types="node" /> -- and looked like this:

import { Exception } from '@microsoft.azure/tasks';
import * as fs from 'fs';
// ... etc ... 

I'm not sure what this split was intent on doing, but it had a side-effect with the typescript compiler, which causes dependency problems

I rolled back and hard-coded 10.12.19 and rebuilt and everything worked as expected.

[rbuckton]

Unfortunately our current logic for automatically detecting whether to emit a type-reference directive when writing declarations is rather limited at the moment and doesn't look further than the entry point file. As an alternative to pinning your version of the node declarations, you could also address this by adding a "types": ["node"] entry to your tsconfig.json. We will continue to investigate this further.

[fearthecowboy]

I actually do have that in my base tsconfig file:

   "types": [
      "node"
    ],

and each of the projects extends the base one:

 "extends": "../../tsconfig.json",

Which is why seeing this shocked me a bit... even if that <reference ...> wasn't emitted, the types in the tsconfig should have covered it.

Hmmm.

[SimonSchick]

Weird enough I cannot repro this issue, are you sure your tsconfig files are 100% correctly loaded?

[rbuckton]

I have been able to reproduce the primary issue (where the types reference is lost), but have not yet been able to reproduce the tsconfig.json issue.

[SimonSchick]

Can you please provide a minimal repro?

[rbuckton]

main.ts

/// <reference types="node" />
import { FSWatcher } from "fs";
export function f() {
    return {} as FSWatcher;
}

tsconfig.json

{
    "compilerOptions": {
        "target": "es2015",
        "module": "commonjs",
        "declaration": true
    }
}

With @types/node installed.

[rbuckton]

I am testing a fix for the type reference directive emit in the compiler now.

[SimonSchick]

Oh I misunderstood!
I always added node to types as that's a habit with all runtime types (mocha, etc).

[Flarna]

The node types were split into two files already before (index.d.ts and inspector.d.ts). What exactly causes that it's a problem pops up now?
Is it because index.d.ts holds just references? Would it be enough to move some parts (e.g. namespace NodeJS) into index.d.ts?

@fearthecowboy The reason for the split is maintainability. Node typings are doomed to support ancient typescript versions back till 2.1. To allow use of newer typescript features (for users of newer typescript) and to unblock new typescript features parts of node typings need to be duplicated. To avoid full duplication a split into modules was done.

[rbuckton]

This was an existing issue that was harder to spot with only two files. You would only hit it if you only ever ended up with types from "inspector" in your declaration file, and more often than not you had other types like Buffer or EventEmitter in your output. With everything in its own file, this went from an outside possibility to a guarantee.

Unfortunately, just moving some types back isn't a solution, as we would be constantly chasing down the same issue for each new type that wasn't in index.d.ts. Normally, adding a "types" option to the tsconfig files for consuming projects would be enough, I am not certain why that is not working in this case as I am not yet able to reproduce that specific problem.

[rbuckton]

@fearthecowboy: I've merged a fix for this in TypeScript's master branch. Would you be willing to test this with tomorrows nightly dev build?

[fearthecowboy]

You bet.

[rbuckton]

@fearthecowboy: You mentioned you were still seeing this if you had "types": ["node"] in your tsconfig.json files. How is your project structured?

One possibility is that one of your projects doesn't have a devDependency for @types/node so it is unable to find the reference, even if it is in your "types" entry in tsconfig.json.

[rosskevin]

One possibility is that one of your projects doesn't have a devDependency for @types/node so it is unable to find the reference, even if it is in your "types" entry in tsconfig.json.

@rbuckton adding @types/node to my devDependencies solved my problem in our monorepo. The oddity here though is the specific project it was failing on was targeted at esnext for the browser, not node - though we do have two other projects in the monorepo targeting node. So strangely enough I was able to continue - just not sure it makes sense to need to add @types/node in dev dependencies for a browser project. Here's a snapshot of the deps for the project that was failing to build and now does:

{
  "devDependencies": {
    "@types/node": "^10.12.21",
    "jest": "^24.1.0"
  },
  "dependencies": {
    "@types/apollo-upload-client": "^8.1.1",
    "@types/graphql": "^14.0.5",
    "@types/i18next": "^12.1.0",
    "@types/jest": "^24.0.0",
    "@types/lodash": "^4.14.120",
    "apollo-cache-inmemory": "^1.4.2",
    "apollo-client": "^2.4.12",
    "apollo-link": "^1.2.8",
    "apollo-link-batch-http": "^1.2.8",
    "apollo-link-context": "^1.0.14",
    "apollo-link-error": "^1.1.7",
    "apollo-link-logger": "^1.2.3",
    "apollo-upload-client": "^10.0.0",
    "cross-fetch": "^3.0.1",
    "graphql": "^14.1.1",
    "i18next": "^14.1.1",
    "js-cookie": "^2.2.0",
    "lodash": "^4.17.11"
  }
}

Hopefully this information is useful - not sure it is.