grpc / grpc-node

gRPC for Node.js
https://grpc.io
Apache License 2.0
4.45k stars 645 forks source link

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

Open cliffordfajardo opened 10 months ago

cliffordfajardo commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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

cliffordfajardo commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 10 months ago

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 commented 9 months ago

Same problem for newest SveleKit.

sebastianrueckerai commented 8 months ago

Same problem with firebase-functions!

acomagu commented 7 months ago

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

cliffordfajardo commented 7 months ago

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 commented 6 months ago

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

dotboris commented 6 months ago

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 https://github.com/dcodeIO/long.js/commit/453de51b6aa9123bf18d0856ac220aff79920a92 changes the way the module is exported.

Under the hood, @grpc/proto-loader re-exports Long from the long lib: https://github.com/grpc/grpc-node/blob/d9c26724a5c85e33f56510600f054856850de7df/packages/proto-loader/src/index.ts#L25-L27

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: https://github.com/dcodeIO/long.js/issues/109. I see open PRs trying to address this but they have not been reviewed for a while now.

manzaloros commented 4 months ago

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

I can confirm that @samhh 's suggestion worked for me as well:

...
  "moduleResolution": "Bundler",
    "resolveJsonModule": true,
    "target": "ES2022",
    "module": "Preserve",
...