grpc-node icon indicating copy to clipboard operation
grpc-node copied to clipboard

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

Open cliffordfajardo opened this issue 2 years ago • 15 comments

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 avatar Nov 15 '23 15:11 cliffordfajardo

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

cliffordfajardo avatar Nov 15 '23 16:11 cliffordfajardo

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 avatar Nov 15 '23 17:11 murgatroid99

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

samhh avatar Nov 24 '23 16:11 samhh

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

cliffordfajardo avatar Nov 26 '23 14:11 cliffordfajardo

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 avatar Dec 01 '23 22:12 davidfiala

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.

davidfiala avatar Dec 01 '23 22:12 davidfiala

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.

manzaloros avatar Dec 06 '23 02:12 manzaloros

Same problem for newest SveleKit.

mpiorowski avatar Dec 19 '23 22:12 mpiorowski

Same problem with firebase-functions!

sebastianrueckerai avatar Jan 24 '24 21:01 sebastianrueckerai

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

acomagu avatar Mar 06 '24 09:03 acomagu

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

cliffordfajardo avatar Mar 07 '24 19:03 cliffordfajardo

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

samhh avatar Mar 15 '24 15:03 samhh

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 [email protected] to [email protected] or [email protected]. 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 [email protected]. 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.

dotboris avatar Mar 26 '24 11:03 dotboris

"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",
...

manzaloros avatar May 20 '24 16:05 manzaloros

The problem is that the advice given above is fundamentally wrong. you will not be able to build a valid esm package by specifying "moduleResolution": "Bundler"... . Grpc-js itself should be an esm and long should be an esm too.

polRk avatar Jan 10 '25 09:01 polRk