-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 class not compatible with generated message interface #837
Comments
Messages with optional fields will now implement their corresponding interfaces (IMessage) fixes protobufjs#834 fixes protobufjs#837
Messages with optional fields will now implement their corresponding interfaces (IMessage) fixes protobufjs#834 fixes protobufjs#837
Does this issue still exist in master? There have been a couple merges that might, or might not, prevent this. Otherwise, do you have an updated example causing the issue? |
I haven't attempted with master. The earliest I'll be able to test it is Monday. |
I had the same issue in version |
6.8.3 still the same issue. |
This change adds Flow declaration types so that we can check the types of the generated code. Notably: - Adds a dependency to flowgen and calls it during compilation, which takes the `.d.ts` and creates a `.flow.js` based on it. - Passes in the `--force-number` flag to pbjs, which causes it to not emit numbers as possibly being `Long`, since this confuses Flow. This only affects JSDoc declarations, not the generated code. - Passes in the `--force-message` flag to pbjs. Otherwise, this emits invalid types: it tries to create code like this: ```TypeScript interface IMessage { field?: T|null; } class Message implements IMessage { field: T; // notice the lack of optionality / nullness. ... } ``` This violates the Liskov Substitution Principle, since `Message` cannot be passed to any place that accepts an `IMessage`, so TypeScript (correctly) rejects this as bogus. With this flag, the number of places that will accept an `IMessage` instead of a `Message` are limited to constructors, where we do want to have partial messages being passed in. This only affects JSDoc declarations, not the generated code. - Removes all `@implements` annotations from the JSDoc, which takes care of the very last `IMessage`/`Message` confusion. This, together with the above element take care of protobufjs/protobuf.js#837
This change adds Flow declaration types so that we can check the types of the generated code. Notably: - Adds a dependency to flowgen and calls it during compilation, which takes the `.d.ts` and creates a `.flow.js` based on it. - Passes in the `--force-number` flag to pbjs, which causes it to not emit numbers as possibly being `Long`, since this confuses Flow. This only affects JSDoc declarations, not the generated code. - Passes in the `--force-message` flag to pbjs. Otherwise, this emits invalid types: it tries to create code like this: ```TypeScript interface IMessage { field?: T|null; } class Message implements IMessage { field: T; // notice the lack of optionality / nullness. ... } ``` This violates the Liskov Substitution Principle, since `Message` cannot be passed to any place that accepts an `IMessage`, so TypeScript (correctly) rejects this as bogus. With this flag, the number of places that will accept an `IMessage` instead of a `Message` are limited to constructors, where we do want to have partial messages being passed in. This only affects JSDoc declarations, not the generated code. - Removes all `@implements` annotations from the JSDoc, which takes care of the very last `IMessage`/`Message` confusion. This, together with the above element take care of protobufjs/protobuf.js#837 - Forwards calls from `.create()` to `.fromObject()` to avoid breaking existing code (w.r.t. types).
protobuf.js version: 6.8.0
Problem:
Message class instances cannot be assigned to interface typed variable (or function parameter);
Generated TypeScript definitions.
Code resulting in TypeScript Error:
TypeScript Error:
A similar problem was fixed in response to this issue: #826. The types now correctly reflect that an instantiated message class can have undefined fields. But assignment to the interface (by calling
Message.encode
for example) is still impossible.Example generated TypeScript interface which would fix the problem:
The text was updated successfully, but these errors were encountered: