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

Generated TypeScript message definition not compatible with message$Properties definition #826

Closed
bhouser opened this issue Jun 8, 2017 · 7 comments
Labels

Comments

@bhouser
Copy link

bhouser commented Jun 8, 2017

protobuf.js version: 6.7.3
Protobuf version: 3

Generated TypeScript message definition not compatible with message$Properties definition. It's not possible to (for example) pass a Token object as a parameter to Token.encode(token), since the types are incompatible.

Protobuf definition

message Token {
    google.protobuf.Any message = 1;
}

Generated Typescript definitions

type Token$Properties = {
    message?: google.protobuf.Any$Properties;
};

class Token {
    constructor(properties?: core.Token$Properties);
    public message: (google.protobuf.Any$Properties|null);
    public static encode(message: core.Token$Properties, writer?: $protobuf.Writer): $protobuf.Writer;
    public static encodeDelimited(message: core.Token$Properties, writer?: $protobuf.Writer): $protobuf.Writer;
    public static decode(reader: ($protobuf.Reader|Uint8Array), length?: number): core.Token;
    public static decodeDelimited(reader: ($protobuf.Reader|Uint8Array)): core.Token;
    public static verify(message: { [k: string]: any }): string;
}

Typescript error:

Error:(42, 37) TS2345:Argument of type 'Token' is not assignable to parameter of type 'Token$Properties'.
  Types of property 'message' are incompatible.
    Type 'Any$Properties | null' is not assignable to type 'Any$Properties | undefined'.
      Type 'null' is not assignable to type 'Any$Properties | undefined'.

I'm pretty sure it should be possible to assign an object of Type Token to an object of type Token$Properties. This is however impossible, since public message: (google.protobuf.Any$Properties|null); is incompatible with message?: google.protobuf.Any$Properties;. An optional field does not accept types which could be null.

I believe this is the same issue as described here:
#808

@bhouser bhouser changed the title https://github.com/dcodeIO/protobuf.js/issues/808 Generated TypeScript message definition not compatible with message$Properties definition Jun 8, 2017
@dcodeIO dcodeIO added the bug label Jun 9, 2017
@bhouser
Copy link
Author

bhouser commented Jun 9, 2017

I forgot to mention: I have the TypeScript compiler flag strictNullChecks activated. I encountered the issue after activating strictNullChecks in my project. Without that flag, the problem disappears, since in that case null can be assigned to any type.

@bhouser
Copy link
Author

bhouser commented Jun 12, 2017

This is great. Any prediction as to when this will land in an npm release?

@dcodeIO
Copy link
Member

dcodeIO commented Jun 12, 2017

It's on npm as 6.8.0

@bhouser
Copy link
Author

bhouser commented Jun 12, 2017

You're right, it is in 6.8.0. I was under the impression that 6.8.0 was final before I posted this issue. Terribly sorry about overlooking that! Thanks!

@dcodeIO
Copy link
Member

dcodeIO commented Jun 12, 2017

No worries, you're welcome! :)

@Alexendoo
Copy link

The types are still incompatible, example:

ERROR in ./src/registration.ts
(25,37): error TS2345: Argument of type 'Link' is not assignable to parameter of type 'ILink'.
  Types of property 'newDevice' are incompatible.
    Type 'INewDevice | null | undefined' is not assignable to type 'INewDevice | undefined'.
      Type 'null' is not assignable to type 'INewDevice | undefined'.

It's because of the presence of null rather than the lack of undefined

@bhouser
Copy link
Author

bhouser commented Jun 12, 2017

@dcodeIO I think Alexendoo is right. The change you made doesn't solve the problem I posted, even though the types are now more accurate: the constructor for a given message can leave fields undefined, which is now reflected in the types.

But yes, the presence of null in the class prevents assignment to a variable with the interface type.

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