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

Autogenerated code erro: Cannot use namespace 'Long' as a type #2617

Open
cliffordfajardo opened this issue Nov 15, 2023 · 13 comments
Open

Autogenerated code erro: Cannot use namespace 'Long' as a type #2617

cliffordfajardo opened this issue Nov 15, 2023 · 13 comments

Comments

@cliffordfajardo
Copy link

cliffordfajardo commented Nov 15, 2023

Problem description

The autogenerated type code from @grpc/grpc-js is generating code that is not valid typescript.

Example Error

For example, the Long that is imported causes an error:
CleanShot 2023-11-15 at 07 18 39@2x

I edited the autogenerated file by using typeof Long further below in the code & that made the typescript error go away.
This works because Namespaces can be converted to types using the typeof keyword.

CleanShot 2023-11-15 at 07 19 34@2x

Reproduction steps

Work in progress

This is what my settings for generating types looks like using @grpc/proto-loader
https://www.npmjs.com/package/@grpc/proto-loader#generating-typescript-types

yarn run proto-loader-gen-types \
  -I $PROTOS_DESTINATION_DIR/serviceProto \
  -I $PROTOS_DESTINATION_DIR \
  --keepCase \
  --longs=String \
  --enums=String \
  --defaults \
  --oneofs \
  --grpcLib=@grpc/grpc-js \
  --outDir=$PROTOS_DESTINATION_DIR \
  $PROTOS_DESTINATION_DIR/serviceProto/proto/com/linkedin/euler/controlplane/*.proto

Environment

OS Name: Sonomo 14.1.1
Node version 20.5.0
Typrscript: 5.0.4
@grpc/grpc-js": "1.9.9",
@grpc/proto-loader": "0.7.10",

@cliffordfajardo
Copy link
Author

cliffordfajardo commented Nov 15, 2023

Using --longs=Number also generated the same type error; its missing typeof Long
Namespaces can be converted to types using the typeof keyword

CleanShot 2023-11-15 at 08 06 17@2x

Just for the sake of it, I upgraded to typescript latest 5.2.2 and that doesnt change anything either; same generated code with type error

@murgatroid99
Copy link
Member

I can't reproduce this with my own generated code with the same version of both proto-loader and TypeScript. Can you create a repository with a complete, self-contained reproduction?

@samhh
Copy link

samhh commented Nov 24, 2023

I started seeing this error once I changed moduleResolution from Node10 to Bundler. It looks like others have been affected by this error too: firebase/firebase-js-sdk#7279

@cliffordfajardo
Copy link
Author

I started seeing this error once I changed moduleResolution from Node10 to Bundler. It looks like others have been affected by this error too: firebase/firebase-js-sdk#7279

Ahhh interesting! Actually this error started occurring in our codebase at the time when we updated our moduleResolution from node to Bundler! I will check our code & try it out to verify this in our codebase

davidfiala added a commit to davidfiala/badbundle that referenced this issue Dec 1, 2023
@davidfiala
Copy link
Contributor

davidfiala commented Dec 1, 2023

I can't reproduce this with my own generated code with the same version of both proto-loader and TypeScript. Can you create a repository with a complete, self-contained reproduction?

@murgatroid99

https://github.com/davidfiala/badbundle

$ npm install
$ npm run build

for errors like

node_modules/@grpc/grpc-js/build/src/generated/google/protobuf/Int64Value.d.ts:3:34 - error TS2709: Cannot use namespace 'Long' as a type.
3     'value'?: (number | string | Long);
node_modules/@grpc/grpc-js/build/src/generated/google/protobuf/Timestamp.d.ts:3:36 - error TS2709: Cannot use namespace 'Long' as a type.
3     'seconds'?: (number | string | Long);

etc...

Edit:

If I replace in the generated code: import type { Long } from '@grpc/proto-loader'; with this instead: import Long from 'long'; and add a dependency to long in grpc-js's package.json, then it will compile under "target": "es2022", "module": "esnext", "moduleResolution": "Bundler",. So to my comment below, it seems that we can skip the re-export of long and depend on it directly.

@davidfiala
Copy link
Contributor

davidfiala commented Dec 1, 2023

Worth pointing out that modifying my example above with:

    "module": "nodenext",
    "moduleResolution": "NodeNext",

yields

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 in node_modules/long/umd/index.d.ts:1

I wonder if it could help to directly import Long from the 'long' library everywhere instead of how it is currently being re-exported from @grpc.

I've also seen other projects like ts-proto attempt to allow the user to configure how generated TS code should handle 64-bit longs. Personally, I'd like to be optionally free of long.js as it has caused me dependency problems in the past, and I suspect that /some/ folks would like the option to treat 64bit ints either opaquely or as strings, thus no need for long.js.

@manzaloros
Copy link

manzaloros commented Dec 6, 2023

I also have this issue with module resolution: "Bundler". Going back to "Node" fixes the issue temporarily.

That isn't a great fix, though, because "Bundler" is the default module resolution for new RemixJS apps.

@mpiorowski
Copy link

Same problem for newest SveleKit.

@sebastianrueckerai
Copy link

Same problem with firebase-functions!

@acomagu
Copy link

acomagu commented Mar 6, 2024

I'm going to use the skipLibCheck flag to work around this problem...

@cliffordfajardo
Copy link
Author

cliffordfajardo commented Mar 7, 2024

Haven't tried this out yet (I'm on mobile) but typescript team announced new improvements to module resolution "Bundler" yesterday in typescript 5.4

need to check if this helps:

https://devblogs.microsoft.com/typescript/announcing-typescript-5-4/#support-for-require-calls-in---moduleresolution-bundler-and---module-preserve

@samhh
Copy link

samhh commented Mar 15, 2024

"module": "preserve" fixes this issue with "moduleResolution": "bundler" for me.

@dotboris
Copy link

I've hit this issue as well and I've done some digging. I believe that I have a better idea of the root cause of the issue. This issue happens when you upgrade from long@5.2.1 to long@5.2.2 or long@5.2.3. This commit dcodeIO/long.js@453de51 changes the way the module is exported.

Under the hood, @grpc/proto-loader re-exports Long from the long lib:

import Long = require('long');
export { Options, Long };

I've worked around the issue on my end by downgrading to long@5.2.1. I'm using yarn@3 so I was able to do so through the resolutions field in package.json:

{
  "resolutions": {
    "@grpc/proto-loader@npm:0.7.10/long": "5.2.1",
    "protobufjs@npm:7.2.5/long": "5.2.1",
    "protobufjs@npm:7.2.6/long": "5.2.1"
  }
}

It feels like the deeper problem is in long which seems to be trying to solve import related issues: dcodeIO/long.js#109. I see open PRs trying to address this but they have not been reviewed for a while now.

Zemnmez added a commit to zemn-me/monorepo that referenced this issue May 1, 2024
This is necessitated as a fix for [pr5226], due to [grpc-node issue 2617].


[grpc-node issue 2617]: grpc/grpc-node#2617
[pr5226]: #5226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants