import Long from 'long'; even when long option is number

[doctorpangloss]

Importing long is emitted even though the long format is number.

Version: 1.81.3
options:

        option "outputClientImpl=grpc-web"
        option "esModuleInterop=true"
        option "returnObservable=false"
        option "addGrpcMetadata=true"
        option "forceLong=number"
        option "oneof=unions"
        option "outputJsonMethods=false"
        option "lowerCaseServiceMethods=true"

[stephenh]

Yeah, that's surprising, can you paste a snippet of the code that has Long in it?

[Jake-RoundrockIO]

Here's an outputted file including Long despite the inclusion of --ts_proto_opt=forceLong=number. All it appears to do it unnecessarily configure protobufjs.

/* eslint-disable */
import { GrpcMethod, GrpcStreamMethod } from '@nestjs/microservices';
import Long from 'long';
import _m0 from 'protobufjs/minimal';
import { Observable } from 'rxjs';
import { Metadata } from '@grpc/grpc-js';

export const protobufPackage = 'hero';

export interface HeroById {
  id: number;
}

export interface Hero {
  id: number;
  name: string;
}

export const HERO_PACKAGE_NAME = 'hero';

export interface HeroesServiceClient {
  findOne(
    request: HeroById,
    metadata: Metadata,
    ...rest: any
  ): Observable<Hero>;
}

export interface HeroesServiceController {
  findOne(
    request: HeroById,
    metadata: Metadata,
    ...rest: any
  ): Observable<Hero>;
}

export function HeroesServiceControllerMethods() {
  return function (constructor: Function) {
    const grpcMethods: string[] = ['findOne'];
    for (const method of grpcMethods) {
      const descriptor: any = Reflect.getOwnPropertyDescriptor(
        constructor.prototype,
        method,
      );
      GrpcMethod('HeroesService', method)(
        constructor.prototype[method],
        method,
        descriptor,
      );
    }
    const grpcStreamMethods: string[] = [];
    for (const method of grpcStreamMethods) {
      const descriptor: any = Reflect.getOwnPropertyDescriptor(
        constructor.prototype,
        method,
      );
      GrpcStreamMethod('HeroesService', method)(
        constructor.prototype[method],
        method,
        descriptor,
      );
    }
  };
}

export const HEROES_SERVICE_NAME = 'HeroesService';

if (_m0.util.Long !== Long) {
  _m0.util.Long = Long as any;
  _m0.configure();
}

Here is the .proto file it was created from:

syntax = "proto3";

package hero;

service HeroesService {
  rpc FindOne (HeroById) returns (Hero) {}
}

message HeroById {
  int32 id = 1;
}

message Hero {
  int32 id = 1;
  string name = 2;
}

Version: 1.82.2
Useage:

grpc_tools_node_protoc \
    --plugin=./node_modules/.bin/protoc-gen-ts_proto \
    --ts_proto_opt=env=node \
    --ts_proto_opt=useOptionals=true \
    --ts_proto_opt=useDate=true \
    --ts_proto_opt=forceLong=number \
    --ts_proto_opt=returnObservable=true \
    --ts_proto_opt=addGrpcMetadata=true \
    --ts_proto_opt=addNestjsRestParameter=true \
    --ts_proto_opt=nestJs=true \
    --ts_proto_opt=outputServices=grpc-js \
    --ts_proto_opt=esModuleInterop=true \
    --ts_proto_out="${OUT_DIR}" \
    -I=./proto ./proto/**/*.proto

[stephenh]

Ah, you don't have encode/decode methods...

You're using NestJS to encoding & decoding, do you know if it uses the long library internally?

For ts-proto itself, even if you use forceLong=number, ts-proto's encode & decode methods still need to internally use it, to read/write the bytes on the wire, and then ts-proto "manually" converts the long to a number for you.

So that's why ts-proto is always init'ing this long library, is that even if you want numbers, it needs to use long to get those numbers.

The wrinkle you have is that, with nestJs=true, you're telling ts-proto to not generate its encode/decode methods, which is fine...

Are you 100% sure the NestJS doesn't use long internally for its number support, similar to how ts-proto would?

If it does, we should probably leave this code here.

If not, we could omit it.

I'm tempted to assert it's not really doing any harm?

[aSemy]

I'd like to be able to generate just Typescript interfaces without any dependencies or imports at all. At the moment the generated file include

import Long from "long";
import _m0 from "protobufjs/minimal";

and

if (_m0.util.Long !== Long) {
  _m0.util.Long = Long as any;
  _m0.configure();
}

even though the interfaces all look something like this

export interface ServerLogRecord {
  version: string;
  tick: number;
  eventType: string;
  data?: DataObject;
}

export interface DataObject {
  objectName: string;
  variant?: ObjectVariant| undefined;
}

So it would be really great if there was a way to disable unecessary imports.

I'm using protoc 3.19.1, ts-proto 1.83.3, and these options

  --ts_proto_opt=useOptionals=true
  --ts_proto_opt=outputServices=false
  --ts_proto_opt=outputJsonMethods=false
  --ts_proto_opt=outputEncodeMethods=false
  --ts_proto_opt=forceLong=number
  --ts_proto_opt=esModuleInterop=true
  --ts_proto_opt=exportCommonSymbols=false

[stephenh]

Hey @aSemy , yeah, that makes sense; protobufjs/minimal import depends on long to correctly handle 64-bit numbers:

https://protobufjs.github.io/protobuf.js/#compatibility

So anytime forceLong=number is used, we output that dependency. You can use forceLong=string to use strings for 64-bit numbers, which should remove the dependency on the long library.

The other ideal option would be to use Bigints, but ts-proto doesn't support those yet: #64

[aSemy]

Replacing --ts_proto_opt=forceLong=number with --ts_proto_opt=forceLong=string produces the same result.

[stephenh]

Hm, yeah, that does seem unexpected; we could probably add || forceLong === string to this condition:

https://github.com/stephenh/ts-proto/blob/main/src/main.ts#L295

@aSemy do you want to submit a PR for ^?

[aSemy]

I'm not sure what you mean, I'm not very familiar with Typescript, and I don't understand the existing code. Is this what you mean?

  const longInit = options.onlyTypes
    ? code``
    : code`
      ${disclaimer}
      if (${util}.Long !== ${Long} || forceLong === string) {
        ${util}.Long = ${Long} as any;
        ${configure}();
      }
    `;

[stephenh]

Fwiw I believe #599 has fixed this issue, so I'm going to close it out. Feel free to point out if it hasn't / there are other remaining issues. Thanks @3846masa !