long.js
long.js copied to clipboard
TypeScript errors with moduleResolution nodenext
I am having trouble upgrading a package that recently switched a dependency from long@^4 to long@^5:
node_modules/long/umd/index.d.ts:1:18 - error TS1479: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("../index.js")' call instead.
1 import Long from "../index.js";
Found 1 error(s).
I can suppress the error by skipping library checks in my tsconfig.json file but perhaps this points to a problem with UMD?
Here is my complete tsconfig for reference (with skipLibCheck=false):
{
"compilerOptions": {
"module": "CommonJS",
"resolveJsonModule": true,
"declaration": false,
"declarationMap": false,
"removeComments": true,
"emitDecoratorMetadata": true,
"experimentalDecorators": true,
"target": "es6",
"lib": ["es6", "dom"],
"sourceMap": true,
"allowSyntheticDefaultImports": true,
"outDir": "./dist",
"baseUrl": "./",
"moduleResolution": "nodenext",
"esModuleInterop": true,
"incremental": false,
"skipLibCheck": false,
"noEmit": false,
"strictNullChecks": true,
"noImplicitAny": true,
"strictBindCallApply": true,
"forceConsistentCasingInFileNames": true,
"noFallthroughCasesInSwitch": true,
"allowJs": true,
"strict": false,
"types": ["mocha", "chai", "node"]
},
"include": ["src/**/*", "hardhat.config.cjs"]
}
https://github.com/stephenh/ts-proto/issues/891
Coming from the ts-proto issue, a minimal reproduction is this index.ts:
import Long = require("long")
console.log("Hello", Long);
Works with this minimal tsconfig (tsc compiles cleanly):
{
"compilerOptions": {
"lib": ["ESNext", "dom"],
"module": "commonjs",
"skipLibCheck": false,
"target": "esnext",
"strict": true
}
}
But breaks when moduleResolution: nodenext is added:
{
"compilerOptions": {
"lib": ["ESNext", "dom"],
"module": "commonjs",
"moduleResolution": "nodenext",
"skipLibCheck": false,
"target": "esnext",
"strict": true
}
}
My guess is that @rjwalters has some dependencies in his node_modules/ using the older import Long = require("long") syntax, which seems to force TypeScript into "importing the UMD module" (which seems reasonable), but then with his project's moduleResolution: nodenext turned on, it seems to realize/think that the umd/index.d.ts importing ../index.js/d.ts is invalid.
...ah, yes, here it is:
"Finally, it’s worth noting that the only way to import ESM files from a CJS module is using dynamic import() calls. This can present challenges, but is the behavior in Node.js today."
https://www.typescriptlang.org/docs/handbook/esm-node.html
Fwiw I think the umd/index.d.ts should just be a copy/paste of index.d.ts (with the = change), similar to ./index.js and ./umd/index.js fundamentally being separate files, and that will probably resolve this issue.
As-is, we've got two totally separate JS files (umd/esm), but are trying to reuse the types across the two, and TS's stricter moduleResolution: nodenext setting is realizing that's not supported.
Granted, @rjwalters can a) use skipLibCheck: true to have tsc not bother double-checking long.js's types against his own stricter tsconfig settings, or b) not use moduleResolution: nodenext, but both of those seem somewhat reasonable.
Fwiw @rjwalters you might consider re-titling this issue (if you're allowed to as the reporter) to "TypeScript errors with moduleResolution nodenext" b/c I believe that is the core issue here.
Granted, I think it does take a library using import Long = require("..."), but in long v4/CommonJS that was the most kosher way to import a directly-exported const.
I met similar error before, and #124 fixed this for me. Not sure if it works for you
Thanks @fs-eire ! I think that's exactly what we need. Thanks for creating the PR!
The PR from @fs-eire works for me.
Running into the same issue. Anyone has an idea as to why this is breaking only now? I've been running this code for month wihtout isues
Typescript 5.2 has been released and you can no longer mix-and-match.
I upgraded my repo to Typescript 5.2, Node 18 etc... and got the same error as @rjwalters
The fix in #124 works but it probably not going to be a solution in production environments.
Is there a path to fixing this that I could look at @dcodeIO or is this a pretty big change for the repo overall?
Perhaps if the UMD typings are generated by build / excluded from Git, with that one line automatically amended, instead of manually duplicating these?
Perhaps if the UMD typings are generated by build / excluded from Git, with that one line automatically amended, instead of manually duplicating these?
Yes. The recommended way is to copy the file in npm prepack script. We don't need the duplicated index.d.ts file in the code base. We only need it in the NPM package
Perhaps if the UMD typings are generated by build / excluded from Git, with that one line automatically amended, instead of manually duplicating these?
I guess so.
What would be the problems if this solution was put in place?
I'd love to open a PR with some kind of fix, but I'm not sure of the problems that might happen if UMD typings are excluded?
This also affects node16 (which is currently an alias of nodenext) which has strong language about not using any other setting with node:
Because node16 and nodenext are the only module options that reflect the complexities of Node.js’s dual module system, they are the only correct module options for all apps and libraries that are intended to run in Node.js v12 or later, whether they use ES modules or not.
Also, skipLibCheck sucks and I don't like using it if I can avoid it :D
Is this getting resolved soon??
It affects Firebase Functions in some cases 💀
Any news on this issue?
Somehow the codebase owner is unwilling to merge my PR #124. I am currently using the following workaround in my package.json to make typescript happy:
"scripts": {
...
"preprepare": "node -e \"require('node:fs').copyFileSync('./node_modules/long/index.d.ts', './node_modules/long/umd/index.d.ts')\"",
...
Somehow the codebase owner is unwilling to merge my PR #124. I am currently using the following workaround in my package.json to make typescript happy:
"scripts": { ... "preprepare": "node -e \"require('node:fs').copyFileSync('./node_modules/long/index.d.ts', './node_modules/long/umd/index.d.ts')\"", ...
Thanks for providing this workaround. Hopefully the PR get's revised/merged, it's been months. protobufjs also has this package as a dependency so it affects lots of projects