Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript errors with moduleResolution nodenext and long.js #891

Closed
rjwalters opened this issue Jul 22, 2023 · 11 comments
Closed

TypeScript errors with moduleResolution nodenext and long.js #891

rjwalters opened this issue Jul 22, 2023 · 11 comments

Comments

@rjwalters
Copy link

rjwalters commented Jul 22, 2023

The most recent release bumps the dependency for ts-proto-descriptors to 1.14, which in turn causes the long package dependency to go from 4 to 5. I am having commonjs/ecmascript problems that seem to be related to long 5 building my nest js project...

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).

reverting to 1.151 fixes the problem

@rjwalters
Copy link
Author

perhaps related to this?

dcodeIO/long.js#109

@stephenh
Copy link
Owner

stephenh commented Jul 22, 2023

The long.js#109 insinuates this was fixed in their 5.2.3 release, which technically we only depended on 5.0.0, so @rjwalters I just released a fix that tries depending on ^5.2.3; can you see if that works?

Fwiw I kind of doubt it will; that error message is interesting:

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.

The error is coming from within long's own umd/index.d.ts, and afaict is asserting that their UMD fallback, which is currently just two lines:

import Long from "../index.js";
export = Long;

Should actually be:

import Long = require("../index.js");
export = Long;

Which kind of makes sense, b/c that import Long = require(...) is an old TS-ism for importing CommonJS-files-that-export-a-single-const (from here.

...although still, that says "if the file you're importing uses export = ", but what this UMD fallback is trying to do, is import from the index.js/.d.ts that is using export default Long, which in theory the very thing that the UMD fallback is supposed be working around.

@rjwalters questions would be:

  1. What version of TS are you using
  2. Do you have esModuleInterop on in your tsconfig.json
  3. Can you try just hand-editing that node_modules/long/umd/index.d.ts file to use the const Long = import(...) and see if that works?

All said, I'm not sure why this is not working for you, but is working fine for ts-proto's test suite. Any insights you can bring in that regard will be helpful in resolving this (i.e. so far this "works for me" 🤷 ).

@stephenh
Copy link
Owner

Okay, I can reproduce this issue with:

  • module: nodenext in tsconfig, and no skipLibCheck
  • Importing long via the old import Long = require("long"); syntax
  • This import Long = require(...) seems to force tsc to use the umd/index.d.ts

So, @rjwalters my guess is that you're not passing esModuleInterop=true to ts-proto as ts_proto_opt? And if you do this, it will probably fix the output?

@rjwalters
Copy link
Author

I can confirm that I have "esModuleInterop": true, in my tsconfig.json file. I am running typescript at (5.1.6).

I changed my protoc command to include the "--esModuleInterop" flag:

protoc --ts_proto_out=. --plugin=node_modules/.bin/protoc-gen-ts_proto --ts_proto_opt=snakeToCamel=true --ts_proto_opt=outputEncodeMethods=false --ts_proto_opt=outputJsonMethods=false --ts_proto_opt=outputClientImpl=false --ts_proto_opt=esModuleInterop=true --ts_proto_opt=fileSuffix=.interface ./src/tokens/v1/tokens.proto

This did not result in any changes in the generated files so the original error still appears when I run "nest build" on my project.

I'm not sure what to try when editing umd/index.d.ts

//import Long from "../index.js";
const Long = import("../index.js");
export = Long;

this creates new errors:

node_modules/long/umd/index.d.ts:2:1 - error TS1046: Top-level declarations in .d.ts files must start with either a 'declare' or 'export' modifier.

2 const Long = import("../index.js");
  ~~~~~
node_modules/long/umd/index.d.ts:2:14 - error TS1254: A 'const' initializer in an ambient context must be a string or numeric literal or literal enum reference.

2 const Long = import("../index.js");
               

@rjwalters
Copy link
Author

here's my full tsconfig.json:

{
  "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"]
}

@stephenh
Copy link
Owner

stephenh commented Jul 22, 2023

Yeah sorry, I'd completely made up const Long = import("../index.js"); and should have typed import Long = require("../index.js");. I was just recalling from memory "something something different import syntax". (I'd edited my comment, but you caught me :-).

This did not result in any changes in the generated files

What do the long imports in the ts-proto-generated files look file?

Since you're using esModuleInterop in your tsconfig file, you want to ensure the ts-proto generated code is using regular import Long from long and not the old import Long = require(long).

@rjwalters
Copy link
Author

I'm not using long in any of my rpc messages -- my service uses only uint32 and string types. The only import in the generated code is for observable (for streaming):

import { Observable } from 'rxjs';

with umd/index.js edited to:

// import Long from "../index.js";
import Long = require("../index.js");
export = Long;

I get the error:

node_modules/long/umd/index.d.ts:2:23 - error TS1471: Module '../index.js' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported with 'require'. Use an ECMAScript import instead.

2 import Long = require("../index.js");

@stephenh
Copy link
Owner

Can you just set skipLibCheck: true? And then tsc won't worry about how correct/incorrect long's internal types are.

@rjwalters
Copy link
Author

setting skipLibCheck: true in my tsconfig.json file does allow me to build and run tests successfully.

All of the other packages I am currently using depend on long@^4 -- maybe there are still some problems to be worked out on long@5... I can open an issue there.

@stephenh
Copy link
Owner

Cool, filing an issue against long.js sgtm.

Fwiw I wonder if they should stop trying to "import the ESM types into the umd/index.d.ts" and just copy/paste the types around instead. Granted, umd/index.d.ts would no longer being a 2-liner, but my guess is that it would be less brittle. 🤷

@stephenh
Copy link
Owner

Fwiw I think dcodeIO/long.js#125 is a good description of what's going on, and it's not a ts-proto issue, so I'm going to close this specific issue out.

@stephenh stephenh changed the title complications upgrading to 1.156 TypeScript errors with moduleResolution nodenext and long.js Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants