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

How to disable option addGrpcMetadata=true for grpc-web impl? #270

Closed
BatyrSeven opened this issue Apr 6, 2021 · 6 comments
Closed

How to disable option addGrpcMetadata=true for grpc-web impl? #270

BatyrSeven opened this issue Apr 6, 2021 · 6 comments
Labels

Comments

@BatyrSeven
Copy link

I think there is no more possibility to disable addGrpcMetadata=true option if having option outputClientImpl=grpc-web. Probably, this issue was introduced in #259

Reproduction:

ts-proto version: 1.79.1

proto:

service DemoService {
    rpc GetBasic (QueryById) returns (BasicInfo) {}
    rpc Insert (Demo) returns (Demo) {}
}

options:

protoc --plugin=node_modules/ts-proto/protoc-gen-ts_proto \
    --ts_proto_opt=outputClientImpl=grpc-web \
    --ts_proto_out=${OUT_DIR} \
    ./*.proto

output:

export interface DemoService {
  GetBasic(request: DeepPartial<QueryById>, metadata?: Metadata): Promise<BasicInfo>;
  Insert(request: DeepPartial<Demo>, metadata?: Metadata): Promise<Demo>;
}

export class DemoServiceClientImpl implements DemoService {
  private readonly rpc: Rpc;

  constructor(rpc: Rpc) {
    this.rpc = rpc;
  }

  GetBasic(request: DeepPartial<QueryById>, metadata?: grpc.Metadata): Promise<BasicInfo> {
    return this.rpc.unary(DemoServiceGetBasicDesc, QueryById.fromPartial(request), metadata);
  }

  Insert(request: DeepPartial<Demo>, metadata?: grpc.Metadata): Promise<Demo> {
    return this.rpc.unary(DemoServiceInsertDesc, Demo.fromPartial(request), metadata);
  }
}

The attempt to compile the output ts file results in the error:
Type 'Metadata | undefined' is not assignable to type 'BrowserHeaders | undefined'.

As you can see, we have Metadata from grpc in the DemoService interface and grpc.Metadata from @improbable-eng/grpc-web in the DemoServiceClientImpl class which causes compatibility issues.

Is there any way to disable option addGrpcMetadata=true for if the clientImpl is grpc-web?

@stephenh
Copy link
Owner

stephenh commented Apr 6, 2021

Can you just add addGrpcMetadata=false to the parameters, i.e. this test is passing:

  it('can disabled grpc metadata', () => {
    const options = optionsFromParameter('outputClientImpl=grpc-web,addGrpcMetadata=false');
    expect(options).toMatchObject({
      outputClientImpl: 'grpc-web',
      addGrpcMetadata: false,
    });
  });

It does sound like there is a larger issue here wrt a mismatch between grpc and improbable-eng types ... which, actually I do see that being reproduced locally, weird that didn't cause a test failure on release. I'll work on fixing that root issue as well.

@BatyrSeven
Copy link
Author

@stephenh thank you for your quick response! However, adding addGrpcMetadata=false didn't work for me - it results in the same error.

@bryanmcgrane
Copy link

I'm experiencing the same issue on a fresh install.

@stephenh
Copy link
Owner

stephenh commented Apr 7, 2021

Ah okay, I found it; this bug was introduced accidentally during a refactored while fixing a related bug. :-) I've got a fix + test case in, and it will be out in the next release. Thanks!

stephenh pushed a commit that referenced this issue Apr 7, 2021
## [1.79.2](v1.79.1...v1.79.2) (2021-04-07)

### Bug Fixes

* Use the right Metadata for grpc-web. Fixes [#270](#270). ([#271](#271)) ([640e645](640e645))
@stephenh
Copy link
Owner

stephenh commented Apr 7, 2021

🎉 This issue has been resolved in version 1.79.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@BatyrSeven
Copy link
Author

Awesome, thanks!

zfy0701 added a commit to sentioxyz/ts-proto that referenced this issue Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants