Definitelytyped: @types/node@13 breaks `instanceof Module` checks

[ericyhwang]

This fails to compile under @types/[email protected], and it successfully compiles under @types/[email protected]:

import Module = require('module');
console.log(module instanceof Module);

The compile error under @types/[email protected] is:

$ tsc -v
Version 3.7.4
$ tsc
src/index.ts:2:31 - error TS2359: The right-hand side of an 'instanceof' expression must be of type 'any' or of a type assignable to the 'Function' interface type.

2 console.log(module instanceof Module);
                                ~~~~~~

The instanceof does work at runtime under the newest Node 13:

$ node -v
v13.5.0
$ node --eval "const Module = require('module'); console.log(module instanceof Module);"
true

And tsc can compile it with @types/node@12:

$ npm i --save-dev @types/node@12

+ @types/[email protected]
updated 1 package and audited 323 packages in 2.335s
found 0 vulnerabilities
$ tsc
$ echo $?
0
$ node build/src/index.js
true

• [x] I tried using the @types/xxxx package and had problems.
• [x] I tried using the latest stable version of tsc. https://www.npmjs.com/package/typescript
• [x] I have a question that is inappropriate for StackOverflow. (Please ask any appropriate questions there).
• [x] [Mention](https://github.com/blog/821-mention-somebody-they-re-notified) the authors (see Definitions by: in index.d.ts) so they can respond.
  
  
  
  • Authors: @SimonSchick @andrewbranch
  
  

[ericyhwang]

This PR added the v13 typings:
https://github.com/DefinitelyTyped/DefinitelyTyped/pull/40927

For code under a developer's control, the most compatible workaround I could find is using the exported Module class under the module named 'module'. It works on both v12 and v13 of the types:

  import Module = require('module');
- console.log(module instanceof Module);
+ console.log(module instanceof Module.Module);

On v13, it is possible to use a cleaner import-from syntax. This will not work on v12.

- import Module = require('module');
+ import { Module } from 'module';
  console.log(module instanceof Module);

[stevendavisphoto]

I am having this problem too since 13.1.0 came out. @types/webpack-env is having the same type collision with @types/node.

Adding "@types/node": "12.12.22", to my package.json file for now to fix.

[andrewbranch]

Sorry for the disruption folks. I have a PR up but it needs to be validated by the node typings maintainers.

On v13, it is possible to use a cleaner import-from syntax. This will not work on v12.

I recommend not updating to a named import until we figure out what to do, because it will stop working if/after my PR lands as-is.

[SimonSchick]

I'm always fascinated how quick people switch to the latest typings despite it being a major version change.

[SimonSchick]

Anyways, this change was actually intentional.
Since we are in TS we figured the preferred way to import module would be to use import { Module } vs. require('module') or even import * as Module from 'module', which is why this was shipped in a major version.
Otherwise we have to use nasty declaration merging (doesn't work well with classes) to achieve exportability and this still doesn't allow import { Module } from 'module';.

That being said, I'm open for suggestions to fix this otherwise.

[andrewbranch]

Can you expound on this?

Otherwise we have to use nasty declaration merging (doesn't work well with classes) to achieve exportability

My concern is that with the current typings, the shape of the module is _wrong_, even though the named export Module is right.

I know very little about how module loading works at runtime in node, but it seems like module resolves to two different things depending on whether it’s being required from a CommonJS module or imported from an ES module. It seems like you have partially described the ES module shape (though it’s missing the default export), but that is fundamentally incompatible with the definition of the CommonJS module shape. This is probably something TypeScript is going to have to solve as we think about our support for .mjs and native ES modules, but since modules are still under an experimental flag in node 13, I think the priority right now needs to be that the CommonJS module shape is represented correctly.

It’s also worth noting that with export = Module, under --esModuleInterop, you can do import Module from 'module' which seems to be correct in node --experimental-modules.

[SimonSchick]

import { Module } from 'module';
import * as Module2 from 'module';
import { default as Module3 } from 'module';

console.log(Module);
console.log('=====');
console.log(Module2);
console.log('=====');
console.log(Module3);
console.log('=====');
console.log(import.meta, Module.createRequire(import.meta.url)('module'));

All of these yield the equivalent results (using raw node, no transpile), I'm not sure what you are referring to in terms of resolving different things.

[andrewbranch]

My understanding, which could be wrong, of the ES module spec implemented in node 13 is that

• You cannot require an ES module
• An ES module can default import a CommonJS module, but not destructure its properties with a named import

In user-land code, how would you write a single module that can be imported/required both by

import { X } from './x' // let‘s ignore the question of file extension for a moment

and by

const X = require('./x')

where X is the same object in both forms? As far as I know (and again my knowledge is quite limited here), this can’t be done in a single file; it can only be done by specifying different main and module keys in a package.json file. (Or, I assume, there could be some magic for the node built-in packages like 'module'.)

So yes, I understand that the Module object is the same object no matter how you get it, but it seems that a CommonJS module format and an ES module format coexist here, which TypeScript doesn’t yet have the capability to describe.

[SimonSchick]

@andrewbranch that was usually accomplished similarly to the events.d.ts file (which is basically a nasty hack) but works somewhat transparently to consumers.

My worry with those hacks here is that some consumers may eventually manage to incorrectly depent on the behaviour of the hack itself eg. the internal class/interface.

[andrewbranch]

Interesting, I see. My main point is that

• A perfect representation of both module formats is probably not possible today, but we know there is work to be done in the compiler in this space.
• In the meantime, it’s more important that any import that is _allowed_ be typed _correctly_ than to support additional import kinds.

So while I want import { Module } from 'module' to work, my view is that we can’t have that at the expense of import Module = require('module') being typed incorrectly. I’d like to get other team members’ takes on this before merging my PR though, so as not to cause more churn than necessary.

[JustFly1984]

I still have this issue, please reopen