Typescript: CommonJS arguments shouldn't be treated as globals in node

[jkrems]

TypeScript Version: [email protected]

Search Terms:

• node
• global
• __dirname

Code

// This should fail unless we target CommonJS.
// It could succeed when targeting CommonJS.
const s: string = __dirname;

// This should always succeed when including the node types
const i: number = process.pid;

Expected behavior:

• process (and other node globals) are defined.
• __dirname (and other CJS arguments) aren't defined.

Actual behavior:

• Including @types/node always defines both true globals and CJS arguments as globals.

Playground Link: The playground doesn't seem to easily support adding the node typings but here's a gist: https://gist.github.com/jkrems/f97d0e040f82c3c4120ca8d3f2fe2fff. The gist comes with a failing test case, allowing npm it after clone.

Related Issues:

• The original bug against @types/node: https://github.com/DefinitelyTyped/DefinitelyTyped/issues/42201
• General issue for module support in Typescript code targeting node: https://github.com/microsoft/TypeScript/issues/18442

[kitsonk]

Why wouldn't this be a @types/node issue and should offer a set of CommonJS globals, and a set of ESM globals, depending on what the user is doing. Just like TypeScript doesn't include node libs, it is something a user is asserting.

[jkrems]

@kitsonk I opened this because I got sent here by an issue against @types/node. Since module etc. is part of the target module system (CommonJS) and Typescript itself generates require calls, there's a pretty good case that this isn't an issue of node types. People can target CommonJS without intending to run in node.

If it's an issue with @types/node, then this issue becomes: How can you express CommonJS arguments within @types/node? They aren't globals but afaict that's the only primitive TypeScript currently exposes. It seems easier for TSC to set them than for TSC to add a new kind of scope. But maybe I don't know the Typescript type system well enough and the latter is easy to add. :)

[DanielRosenwasser]

It sounds like you'd need some sort of capability for declaring that certain bindings are only introduced as parameters under CommonJS - but I don't know if that really warrants more complexity. Additionally, whether we like it or not, I'm sure some code is authored with ECMAScript module syntax in a way that assumes __dirname is a global because it assumes it's going to be bundled in Rollup or Webpack, and emitted as a CommonJS library.

My feeling is that users would be better served by a lint rule.

[jkrems]

One of the unfortunate side effects of the current situation is that it sends people in the wrong direction. If a user writes a module (targeting modern node or modern browsers) and they copy&paste some code using require, it tells them "please install @types/node". They do, Typescript tells them "all is well, this is valid code!" but then it fails at runtime. And at that point there's no real signal why things fail.

[kitsonk]

It sounds like you'd need some sort of capability for declaring that certain bindings are only introduced as parameters under CommonJS - but I don't know if that really warrants more complexity.

I agree about not worth the complexity. That essentially runs really close to a type directed emit. The module target impacts some syntax (like export assignment not being allowed, or top level await being allowed) but being able to express different runtime types based on module target feels like using a sledge hammer to crack a nut.

The lint rule is a good idea, but another possible solution, is to have @types/node-es... In theory, @types/node-es could depend upon the right parts of @types/node and with a little re-factoring, it would solve the problem with little maintenance overhead.

[jkrems]

My feeling is that users would be better served by a lint rule.

A lint rule could definitely help advanced users but I'm mostly concerned about beginners. So working around invalid type info using specific linter setups may not scale to those.

Another possible solution, is to have @types/node-es

Potentially - yes. It wouldn't work well in mixed code projects (some CJS, some modules) but I'm not aware of an incremental module-migration strategy for bigger TS projects. So mixed projects may just not be a concern. Having @types/node-es or @types/nodejs (less weird down the line) could be a nice opt-in way so people compiling faux modules from Typescript aren't affected by the change(s).

Another option is to also just punt on this and see how much of a support burden it will be in practice.

[jkrems]

For context: I opened this issue as part of my investigation into tooling support for ES modules. I don't have a specific project where I ran into this. So it's less about fixing a problem I have myself and more about documenting gaps in module support across the ecosystem. :)

[RyanCavanaugh]

There's no way we'd do something this complicated for the sake of a handful of globals that already start with underscores unless it was something people were constantly getting tripped up on. It's not even clear it'd be a correct error when issued, since you may very well be targeting ESNext modules and having a downstream build step convert to CommonJS

[sosoba]

IMHO we should:
1) Keep all common declarations in@types/node but without both require even import.meta.url.
2) Add new @types/node-csj and @types/node-esm packages with thats global declarations