Type issue with ServerReflectionService

[UncleSamSwiss]

As of the latest versions (see below) the following code doesn't work anymore in typescript:

server.add(
  ServerReflectionService,
  ServerReflection(
    fs.readFileSync(path.join('path', 'to', 'protoset.bin')),
    // specify fully-qualified names of exposed services
    [MyService.fullName],
  ),
);

Error message:

Argument of type 'IServerReflectionService' is not assignable to parameter of type 'CompatServiceDefinition'.
  Type 'IServerReflectionService' is not assignable to type 'ServiceDefinition<UntypedServiceImplementation>'.
    Index signature for type 'string' is missing in type 'IServerReflectionService'.

Versions:

npm list nice-grpc
[email protected] /src
├─┬ [email protected]
│ └── [email protected] deduped
└── [email protected]

[UncleSamSwiss]

Remark: reverting to these versions fixes the problem for now:

npm list nice-grpc
[email protected] /src
├─┬ [email protected]
│ └── [email protected] deduped
└── [email protected]

[davidfiala]

Hi! We'll probably need to know more about your build environment, typescript versions, other dep versions, maybe tsconfig, etc.

I added tests which use the same syntax you provided. But I failed to reproduce in #612

See if you can repro locally:

git clone https://github.com/deeplay-io/nice-grpc.git
cd nice-grpc
yarn
yarn build
cd packages/nice-grpc-server-reflection
yarn test

Edit: the new tests have been merged to master, so I updated the commands above to help diagnose if it is local to your host environment or instead your build environment for just your project

[davidfiala]

I accidentally stumbled upon a repro of this while working on another change which would address #610

Unfortunately, I'm having a hard time making a minimal repro from my larger commit in progress so far, but I figured I'd update that I have seen what you mean.

@UncleSamSwiss Can you please post your typescript version, which compiler/bundler you are using, and the command with plugins you are using to generate your ServerReflection proto?

I think it is something to do with the interplay between gRPC-js, typescript version, and the generated proto.

The reflection.proto was generated as:

grpc_tools_node_protoc --js_out=import_style=commonjs,binary:./src/proto --grpc_out=grpc_js:./src/proto --ts_out=grpc_js:./src/proto -I ./proto proto/grpc/reflection/v1alpha/reflection.proto

I've been working on resolving this by generated differently in my commit by rewriting the package to use the nicer generated protos:

grpc_tools_node_protoc --ts_proto_out=./src/proto --ts_proto_opt=outputServices=nice-grpc,outputServices=generic-definitions,useExactTypes=false,esModuleInterop=true -I proto proto/grpc/reflection/v1alpha/reflection.proto

But even if using different generation works for me in this one case, it means that that old generation format is (maybe?) broken in general, larger the scope of server-reflection package.

[davidfiala]

It's been messy, but I was able to make a repro and pushed it to a repos. The only real delta is the yarn version used.

• https://github.com/davidfiala/brokengrpc/tree/yarn1.22.22 Ancient yarn works
• https://github.com/davidfiala/brokengrpc/tree/yarn4.2.2 Modern yarn fails

Repro:

nvm use v18.20.3

git clone https://github.com/davidfiala/brokengrpc
git checkout yarn1.22.22
rm -rf .pnp.* .yarn/ yarn.lock && touch yarn.lock && yarn && yarn prepare && yarn build
yarn test # passes

git checkout yarn4.2.2
rm -rf .pnp.* .yarn/ yarn.lock && touch yarn.lock && yarn && yarn prepare && yarn build
yarn test # will not compile

Partial output:

> [email protected] build:proto
> grpc_tools_node_protoc --plugin=protoc-gen-grpc=${BERRY_BIN_FOLDER}/grpc_tools_node_protoc_plugin --plugin=protoc-gen-ts=${BERRY_BIN_FOLDER}/protoc-gen-ts --js_out=import_style=commonjs,binary:./fixtures --ts_out=grpc_js:./fixtures --grpc_out=grpc_js:./fixtures -I fixtures fixtures/*.proto

 FAIL  src/index.test.ts
  ● Test suite failed to run

    src/index.test.ts:20:14 - error TS2345: Argument of type 'ITestService' is not assignable to parameter of type 'CompatServiceDefinition'.
      Type 'ITestService' is not assignable to type 'ServiceDefinition<UntypedServiceImplementation>'.
        Index signature for type 'string' is missing in type 'ITestService'.

    20   server.add(TestService, {
                    ~~~~~~~~~~~
    src/index.test.ts:21:44 - error TS7031: Binding element 'signal' implicitly has an 'any' type.

    21     async testUnary(request: TestRequest, {signal}) {
                                                  ~~~~~~
    src/index.test.ts:34:13 - error TS2345: Argument of type 'ITestService' is not assignable to parameter of type 'CompatServiceDefinition'.

    34     .create(TestService, channel);
                   ~~~~~~~~~~~
    src/index.test.ts:36:26 - error TS2339: Property 'testUnary' does not exist on type 'RawClient<ServiceDefinition | FromGrpcJsServiceDefinition<ServiceDefinition<UntypedServiceImplementation>> | FromTsProtoServiceDefinition<...>, DeadlineOptions>'.
      Property 'testUnary' does not exist on type 'RawClient<FromGrpcJsServiceDefinition<ServiceDefinition<UntypedServiceImplementation>>, DeadlineOptions>'.

    36   const promise = client.testUnary(new TestRequest(), {
                                ~~~~~~~~~
    src/index.test.ts:50:14 - error TS2345: Argument of type 'ITestService' is not assignable to parameter of type 'CompatServiceDefinition'.

    50   server.add(TestService, {
                    ~~~~~~~~~~~

[davidfiala]

where /tmp/good is the 1.22.22 version
where /tmp/bad is the 4.2.2 version

result: fixtures, lib, src are all the same

/tmp/good$ find lib src fixtures -type f -exec md5sum {} + | sort
3a4a861fbea2518cf9a440808b346621  fixtures/test_grpc_pb.d.ts
66aff8f8d71567f49e9577dc4c1f49c1  lib/index.d.ts
8b1e5b62eff5763906ceca73b156bfc0  lib/index.js.map
9c5ad733b8b97daf5696d9cde9ce2fda  src/index.test.ts
a20b8068c4330d10512b5450310e76d8  fixtures/test_grpc_pb.js
b7769be5457cced460a18ec43b32610e  fixtures/.gitignore
bc2a1bffcadea38164fa80779967ccc4  lib/index.js
d5bb66dc3053c35a293d8a4164d1e0af  src/index.ts
ded78c134e45245c62abbe8acc8bffa2  fixtures/test_pb.js
ee1bc772e96d65c61bc30ed38fd2f0b9  fixtures/test.proto
ff6c2dae08f729cd7c20cab3674964c3  fixtures/test_pb.d.ts

/tmp/good$ find lib src fixtures -type f -exec md5sum {} + | sort | md5sum
95633bad153a98ae632796d40804db0f  -

/tmp/bad$ find lib src fixtures -type f -exec md5sum {} + | sort
3a4a861fbea2518cf9a440808b346621  fixtures/test_grpc_pb.d.ts
66aff8f8d71567f49e9577dc4c1f49c1  lib/index.d.ts
8b1e5b62eff5763906ceca73b156bfc0  lib/index.js.map
9c5ad733b8b97daf5696d9cde9ce2fda  src/index.test.ts
a20b8068c4330d10512b5450310e76d8  fixtures/test_grpc_pb.js
b7769be5457cced460a18ec43b32610e  fixtures/.gitignore
bc2a1bffcadea38164fa80779967ccc4  lib/index.js
d5bb66dc3053c35a293d8a4164d1e0af  src/index.ts
ded78c134e45245c62abbe8acc8bffa2  fixtures/test_pb.js
ee1bc772e96d65c61bc30ed38fd2f0b9  fixtures/test.proto
ff6c2dae08f729cd7c20cab3674964c3  fixtures/test_pb.d.ts

/tmp/bad$ find lib src fixtures -type f -exec md5sum {} + | sort | md5sum
95633bad153a98ae632796d40804db0f  -

Given that the files are the same, this is a problem with the packages installed. Possibly looking at typescript or ts-jest in particular. Attaching lock files and a diff between them, after cleaning up and sorting each.

yarn-lock-diffed-version-info-only.diff.txt
yarn-lock-1.22.22.txt
yarn-lock-4.2.2.txt

The one that stands out is this @typescript/patch thing mostly because I don't recognize it or know how it works. Highly suspicious given that our only remaining delta between the good/bad version is the yarn version with PnP.

"typescript@patch:typescript@npm%3A^5.4.5#optional!builtin<compat/typescript>":
  version: 5.4.5
  resolution: "typescript@patch:typescript@npm%3A5.4.5#optional!builtin<compat/typescript>::version=5.4.5&hash=5adc0c"
  bin:
    tsc: bin/tsc
    tsserver: bin/tsserver
  checksum: 10c0/db2ad2a16ca829f50427eeb1da155e7a45e598eec7b086d8b4e8ba44e5a235f758e606d681c66992230d3fc3b8995865e5fd0b22a2c95486d0b3200f83072ec9
  languageName: node
  linkType: hard

[davidfiala]

Seems 100% related to Yarn PnP. Turning it off and no errors:

git checkout yarn4.2.2
rm -rf .pnp.* .yarn/ yarn.lock && touch yarn.lock

yarn config set nodeLinker node-modules

yarn && yarn prepare && yarn build
yarn test # now it works

@aikoven I'm wondering if you wouldn't mind taking a look from here. Hopefully this gives you a solid starting point.

You have demonstrably shown your typescript voodoo skills to be excellent by writing nice-grpc itself. And you've managed to create a corner case that broke the wildly popular Yarn PnP. ;) I don't think I am expert enough at the type system to know what we could have caused to occur easily.

If we can make the PoC even smaller, maybe we can report it to the Yarn team directly.

This unfortunately partially blocks another commit I am working on, which does do the dual ESM and CJS output because I depended on upgrading to yarn2+ in it already :/

Let me know what you think if you have cycles to identify the root cause.

[davidfiala]

Oh, and just to clear up a hypothesis: The grpc-js 1.10.x change itself didn't likely cause the breakage the reporter is mentioning.

Instead it was likely this: https://github.com/deeplay-io/nice-grpc/pull/608/files which added "packageManager": "[email protected]". Without that line, the host OS just seems to do whatever it wants. On modern linux distros with fresh node installs, it will pick a modern yarn by default and that cannot compile (hence my other PR in progress to try to modernize this package). At the same time, on the maintainers mac system, it seems to default yarn to 1.22.22 even without that line. By making the package manager explicit we were able to make nice-grpc something all parties on any OS/setup could collaborate on, in theory.

The caveat is that by being explicit, it seems we may have broke someone else. I'm surprised by that though. I've very curious how the OP made this happen. Maybe they brought in nice-grpc via a git submodule. Consuming the compiled NPM distribution I wouldn't have imagined to cause any problem because they aren't compiling anything in nice-grpc. Maybe @UncleSamSwiss can chime in with more details about how they were depping on nice-grpc and setup their build environment.

[UncleSamSwiss]

I will have access to the code again on Tuesday, but what I can already say is:

• it's a nx repo
• we are using npm only (and the npm and Node version (20) didn't change lately
• it's a pretty basic setup apart from the nx part

I'll report more once I can look at the code again.

[UncleSamSwiss]

Here is some additional information:

• Node version: v20.12.0
• npm version: 10.5.0
• TypeScript version: 5.2.2
• Protobuf generation: npx protoc
• Protoc version: 1.1.3
• Missing imports (these are above the code in the issue description):

import {
  CallContext,
  ServerError,
  ServerMiddlewareCall,
  Status,
  createServer,
} from "nice-grpc";
import {
  ServerReflection,
  ServerReflectionService,
} from "nice-grpc-server-reflection";

None of the used types in the failing code are our own and none of them are related to code generated by protoc.

[UncleSamSwiss]

The errors started popping up in our GitHub Actions first (after the merge of the updated dependencies from renovate-bot), so it seems not to be related to a specific local setup (which is Devcontianer btw).

Our tsconfigs are as follows:
tsconfig.json

{
  "extends": "../../tsconfig.base.json",
  "files": [],
  "include": [],
  "references": [
    {
      "path": "./tsconfig.app.json"
    },
    {
      "path": "./tsconfig.spec.json"
    }
  ],
  "compilerOptions": {
    "esModuleInterop": true,
    "resolveJsonModule": true
  }
}

tsconfig.app.json

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "../../dist/out-tsc",
    "module": "commonjs",
    "types": ["node"]
  },
  "exclude": [
    "jest.config.ts",
    "src/**/__mocks__/*.ts",
    "src/**/*.spec.ts",
    "src/**/*.test.ts",
    "src/**/*.mock.ts"
  ],
  "include": ["src/**/*.ts"]
}

tsconfig.base.json

{
  "compileOnSave": false,
  "compilerOptions": {
    "rootDir": ".",
    "sourceMap": true,
    "declaration": false,
    "moduleResolution": "node",
    "emitDecoratorMetadata": true,
    "experimentalDecorators": true,
    "importHelpers": true,
    "target": "es2015",
    "module": "esnext",
    "lib": ["es2021", "dom"],
    "skipLibCheck": true,
    "skipDefaultLibCheck": true,
    "alwaysStrict": true,
    "strictNullChecks": true,
    "noImplicitAny": true,
    "baseUrl": ".",
    "paths": {
      "<confidential>": ["<confidential>"]
    }
  },
  "exclude": ["node_modules", "tmp", "tools"]
}