-
Notifications
You must be signed in to change notification settings - Fork 773
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
Wrong return values for setters when generating .d.ts typings #917
Comments
I was literally just typing up a ticket for the same thing and this didn't come up in the search when I first looked, I'll just paste in here what I had in case it is helpful 😄 The typescript type definitions for a field setter seem to indicate a fluent interface, where the message object itself is returned (allowing for chaining), when in fact nothing is returned. Here is an Example from a generated UUID type with a simple string export class UUID extends jspb.Message {
getValue(): string;
setValue(value: string): UUID;
serializeBinary(): Uint8Array;
toObject(includeInstance?: boolean): UUID.AsObject;
static toObject(includeInstance: boolean, msg: UUID): UUID.AsObject;
static serializeBinaryToWriter(message: UUID, writer: jspb.BinaryWriter): void;
static deserializeBinary(bytes: Uint8Array): UUID;
static deserializeBinaryFromReader(message: UUID, reader: jspb.BinaryReader): UUID;
} I think that this definition would indicate that you could do something like the following: const uuid: UUID = new UUID().setValue('uuid_string_here'); When in fact, the implementation of this method is as follows: /** @param {string} value */
proto.UUID.prototype.setValue = function(value) {
jspb.Message.setProto3StringField(this, 1, value);
}; Which has a void return type. In the example above, this would result in Obviously, you can still do this in multiple lines as follows: const uuid = new UUID();
uuid.setValue('uuid_string_here'); So this isn't really preventing anyone from doing anything probably, but it is a bit confusing if you are trusting autocomplete 😄 I identified the same commit that @Peeet referenced above, so I think he is correct. I'm new to grpc-web, so I don't know if I can be of any help or not, but figured I would document this here in case some improvement can be made. |
* Continue reading websocket to handle closing * added websocket transport to cancel propagation test (grpc#904) (cherry picked from commit 36be481) * Added cancellation test Co-authored-by: Wiktor Jurkiewicz <[email protected]>
I also have this issue, but only when running with protoc on linux. protoc on mac generates javascript code that matches the typescript definitions (the fluent interface), but when generating protoc on linux, the javascript code does not match the typescript definition. This seems a very major bug to me. |
I ran into the same issue. In my case it was caused by an outdated |
Aloha!
When I generate
.d.ts
typings with any grpc-web version > 1.0.7 all setter-functions now return their class rather thanvoid
.E.g. what I expect:
setToken(value: Uint8Array | string): void;
What is generated:
setToken(value: Uint8Array | string): SaltEntry
I guess this is because of a bug introduced with commit #747 (line 924, 927).
The text was updated successfully, but these errors were encountered: