Update node:cluster module type def to support CommonJS
Please fill in this template.
- [x] Use a meaningful title for the pull request. Include the name of the package modified.
- [x] Test the change in your own code. (Compile and run.)
- [x] Add or edit tests to reflect the change.
- [x] Follow the advice from the readme.
- [x] Avoid common mistakes.
- [ ] Run
npm test <package to test>.
Select one of these and delete the others:
If changing an existing definition:
- [ ] Provide a URL to documentation or source code which provides context for the suggested changes: <
> - [ ] If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
This PR replaces #59058.
Description
The current type definitions for Nodes cluster module are currently incompatible with CommonJS. In other words, the current type definitions will mislead consumers into thinking that they need to consume the default export even though no such export exists.
This PR changes the module's signature to use the exports= syntax consistent with how the events module is structured.
Status quo
Node.js "type": "module
import * as cluster from 'cluster';
if (cluster.isPrimary) {
cluster.fork();
} else {
console.log('Fork succeeded');
}
✅ works
import cluster from 'cluster';
if (cluster.isPrimary) {
cluster.fork();
} else {
console.log('Fork succeeded');
}
✅ works
Transpiled to JS using tsc without esModuleInterop
{
"compilerOptions": {
"module": "CommonJS"
},
"include": ["src"]
}
// src/index.ts
import cluster from 'cluster';
if (cluster.isPrimary) {
cluster.fork();
} else {
console.log('Fork succeeded');
}
Produces:
"use strict";
exports.__esModule = true;
var cluster_1 = require("cluster");
if (cluster_1["default"].isPrimary) {
cluster_1["default"].fork();
}
else {
console.log('Fork succeeded');
}
❌ Does not work
// src/index.ts
import * as cluster from 'cluster';
//@ts-ignore
if (cluster.isPrimary) {
//@ts-ignore
cluster.fork();
} else {
console.log('Fork succeeded');
}
Produces:
"use strict";
exports.__esModule = true;
var cluster = require("cluster");
//@ts-ignore
if (cluster.isPrimary) {
//@ts-ignore
cluster.fork();
}
else {
console.log('Fork succeeded');
}
✅ Works but does not type check without the //@ts-ignore annotations
However, enabling "esModuleInterop" allows this to work once again.
My conclusion is that the status quo either:
- Requires users to enable
esModuleInterop; or - Write code that type checks and transpiles but will fail at runtime; or
- Write code that doesn't type check but works.
I think that this PR addresses those concerns but am by no means an expert in the nuance of definition file authoring.
whats the status of this?
Can we please merge this? I face the same issue and not able to use cluster module without any hacks. At least the core modules should be clean enough to use without hacks like ts-ignore or enabling esModuleInterop.
cc @ggoodman is this PR incomplete? if not can you please mark it as ready for review?
Can we have this merged? It still does not support CommonJS in 2024.😅