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

typescript definitions incorrectly generated? #723

Closed
tamird opened this Issue Mar 24, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@tamird

tamird commented Mar 24, 2017

The error reported by tsc:

src/js/protos.d.ts(18180,19): error TS2420: Class 'NamePart' incorrectly implements interface 'NamePart$Properties'.
  Property 'namePart' is missing in type 'NamePart'.

Looks like the class that implements the type has to enumerate every property. See http://www.typescriptlang.org/play/#src=type%20NamePart%24Properties%20%3D%20%7B%0A%09namePart%3A%20string%3B%0A%09isExtension%3A%20boolean%3B%0A%7D%3B%0A%0Aclass%20NamePart%20implements%20NamePart%24Properties%20%7B%0A%7D.

The generated definition excerpt as of a1f23e0:

        namespace UninterpretedOption {

            /**
             * Properties of a NamePart.
             * @typedef google.protobuf.UninterpretedOption.NamePart$Properties
             * @type Object
             * @property {string} namePart NamePart namePart.
             * @property {boolean} isExtension NamePart isExtension.
             */
            type NamePart$Properties = {
                namePart: string;
                isExtension: boolean;
            };

            /**
             * Constructs a new NamePart.
             * @exports google.protobuf.UninterpretedOption.NamePart
             * @implements google.protobuf.UninterpretedOption.NamePart$Properties
             * @constructor
             * @param {google.protobuf.UninterpretedOption.NamePart$Properties=} [properties] Properties to set
             */
            class NamePart implements google.protobuf.UninterpretedOption.NamePart$Properties {

                /**
                 * Constructs a new NamePart.
                 * @exports google.protobuf.UninterpretedOption.NamePart
                 * @implements google.protobuf.UninterpretedOption.NamePart$Properties
                 * @constructor
                 * @param {google.protobuf.UninterpretedOption.NamePart$Properties=} [properties] Properties to set
                 */
                constructor(properties?: google.protobuf.UninterpretedOption.NamePart$Properties);

                /**
                 * Creates a new NamePart instance using the specified properties.
                 * @param {google.protobuf.UninterpretedOption.NamePart$Properties=} [properties] Properties to set
                 * @returns {google.protobuf.UninterpretedOption.NamePart} NamePart instance
                 */
                public static create(properties?: google.protobuf.UninterpretedOption.NamePart$Properties): google.protobuf.UninterpretedOption.NamePart;

                /**
                 * Encodes the specified NamePart message. Does not implicitly {@link google.protobuf.UninterpretedOption.NamePart.verify|verify} messages.
                 * @param {google.protobuf.UninterpretedOption.NamePart$Properties} message NamePart message or plain object to encode
                 * @param {$protobuf.Writer} [writer] Writer to encode to
                 * @returns {$protobuf.Writer} Writer
                 */
                public static encode(message: google.protobuf.UninterpretedOption.NamePart$Properties, writer?: $protobuf.Writer): $protobuf.Writer;

                /**
                 * Encodes the specified NamePart message, length delimited. Does not implicitly {@link google.protobuf.UninterpretedOption.NamePart.verify|verify} messages.
                 * @param {google.protobuf.UninterpretedOption.NamePart$Properties} message NamePart message or plain object to encode
                 * @param {$protobuf.Writer} [writer] Writer to encode to
                 * @returns {$protobuf.Writer} Writer
                 */
                public static encodeDelimited(message: google.protobuf.UninterpretedOption.NamePart$Properties, writer?: $protobuf.Writer): $protobuf.Writer;

                /**
                 * Decodes a NamePart message from the specified reader or buffer.
                 * @param {$protobuf.Reader|Uint8Array} reader Reader or buffer to decode from
                 * @param {number} [length] Message length if known beforehand
                 * @returns {google.protobuf.UninterpretedOption.NamePart} NamePart
                 * @throws {Error} If the payload is not a reader or valid buffer
                 * @throws {$protobuf.util.ProtocolError} If required fields are missing
                 */
                public static decode(reader: ($protobuf.Reader|Uint8Array), length?: number): google.protobuf.UninterpretedOption.NamePart;

                /**
                 * Decodes a NamePart message from the specified reader or buffer, length delimited.
                 * @param {$protobuf.Reader|Uint8Array} reader Reader or buffer to decode from
                 * @returns {google.protobuf.UninterpretedOption.NamePart} NamePart
                 * @throws {Error} If the payload is not a reader or valid buffer
                 * @throws {$protobuf.util.ProtocolError} If required fields are missing
                 */
                public static decodeDelimited(reader: ($protobuf.Reader|Uint8Array)): google.protobuf.UninterpretedOption.NamePart;

                /**
                 * Verifies a NamePart message.
                 * @param {Object.<string,*>} message Plain object to verify
                 * @returns {?string} `null` if valid, otherwise the reason why it is not
                 */
                public static verify(message: { [k: string]: any }): string;

                /**
                 * Creates a NamePart message from a plain object. Also converts values to their respective internal types.
                 * @param {Object.<string,*>} object Plain object
                 * @returns {google.protobuf.UninterpretedOption.NamePart} NamePart
                 */
                public static fromObject(object: { [k: string]: any }): google.protobuf.UninterpretedOption.NamePart;

                /**
                 * Creates a NamePart message from a plain object. Also converts values to their respective internal types.
                 * This is an alias of {@link google.protobuf.UninterpretedOption.NamePart.fromObject}.
                 * @function
                 * @param {Object.<string,*>} object Plain object
                 * @returns {google.protobuf.UninterpretedOption.NamePart} NamePart
                 */
                public static from(object: { [k: string]: any }): google.protobuf.UninterpretedOption.NamePart;

                /**
                 * Creates a plain object from a NamePart message. Also converts values to other types if specified.
                 * @param {google.protobuf.UninterpretedOption.NamePart} message NamePart
                 * @param {$protobuf.ConversionOptions} [options] Conversion options
                 * @returns {Object.<string,*>} Plain object
                 */
                public static toObject(message: google.protobuf.UninterpretedOption.NamePart, options?: $protobuf.ConversionOptions): { [k: string]: any };

                /**
                 * Creates a plain object from this NamePart message. Also converts values to other types if specified.
                 * @param {$protobuf.ConversionOptions} [options] Conversion options
                 * @returns {Object.<string,*>} Plain object
                 */
                public toObject(options?: $protobuf.ConversionOptions): { [k: string]: any };

                /**
                 * Converts this NamePart to JSON.
                 * @returns {Object.<string,*>} JSON object
                 */
                public toJSON(): { [k: string]: any };
            }
        }

dcodeIO added a commit that referenced this issue Mar 24, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Mar 24, 2017

Strange, I am not having this error when running the tests. I changed it to extends instead of implements now - does this solve it?

Edit: Nope, doesn't work. Still no error here, so there must be something wrong with the ts test setup, too.

dcodeIO added a commit that referenced this issue Mar 24, 2017

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Mar 24, 2017

What about this one?

@tamird

This comment has been minimized.

tamird commented Mar 24, 2017

Close but not quite. The fields on the generated class should not end in "$Properties", so

public grants?: cockroach.server.serverpb.TableDetailsResponse.Grant$Properties[];

below should be

public grants?: cockroach.server.serverpb.TableDetailsResponse.Grant[];
            /**
             * Properties of a TableDetailsResponse.
             * @typedef cockroach.server.serverpb.TableDetailsResponse$Properties
             * @type Object
             * @property {Array.<cockroach.server.serverpb.TableDetailsResponse.Grant$Properties>} [grants] TableDetailsResponse grants.
             * @property {Array.<cockroach.server.serverpb.TableDetailsResponse.Column$Properties>} [columns] TableDetailsResponse columns.
             * @property {Array.<cockroach.server.serverpb.TableDetailsResponse.Index$Properties>} [indexes] TableDetailsResponse indexes.
             * @property {Long} [rangeCount] TableDetailsResponse rangeCount.
             * @property {string} [createTableStatement] TableDetailsResponse createTableStatement.
             * @property {cockroach.config.ZoneConfig$Properties} [zoneConfig] TableDetailsResponse zoneConfig.
             * @property {cockroach.server.serverpb.ZoneConfigurationLevel} [zoneConfigLevel] TableDetailsResponse zoneConfigLevel.
             * @property {Long} [descriptorId] TableDetailsResponse descriptorId.
             */
            type TableDetailsResponse$Properties = {
                grants?: cockroach.server.serverpb.TableDetailsResponse.Grant$Properties[];
                columns?: cockroach.server.serverpb.TableDetailsResponse.Column$Properties[];
                indexes?: cockroach.server.serverpb.TableDetailsResponse.Index$Properties[];
                rangeCount?: Long;
                createTableStatement?: string;
                zoneConfig?: cockroach.config.ZoneConfig$Properties;
                zoneConfigLevel?: cockroach.server.serverpb.ZoneConfigurationLevel;
                descriptorId?: Long;
            };

            /**
             * Constructs a new TableDetailsResponse.
             * @exports cockroach.server.serverpb.TableDetailsResponse
             * @implements cockroach.server.serverpb.TableDetailsResponse$Properties
             * @constructor
             * @param {cockroach.server.serverpb.TableDetailsResponse$Properties=} [properties] Properties to set
             */
            class TableDetailsResponse implements cockroach.server.serverpb.TableDetailsResponse$Properties {

                /**
                 * Constructs a new TableDetailsResponse.
                 * @exports cockroach.server.serverpb.TableDetailsResponse
                 * @implements cockroach.server.serverpb.TableDetailsResponse$Properties
                 * @constructor
                 * @param {cockroach.server.serverpb.TableDetailsResponse$Properties=} [properties] Properties to set
                 */
                constructor(properties?: cockroach.server.serverpb.TableDetailsResponse$Properties);

                /**
                 * @type {Array.<cockroach.server.serverpb.TableDetailsResponse.Grant$Properties>|undefined}
                 */
                public grants?: cockroach.server.serverpb.TableDetailsResponse.Grant$Properties[];

                /**
                 * @type {Array.<cockroach.server.serverpb.TableDetailsResponse.Column$Properties>|undefined}
                 */
                public columns?: cockroach.server.serverpb.TableDetailsResponse.Column$Properties[];

                /**
                 * @type {Array.<cockroach.server.serverpb.TableDetailsResponse.Index$Properties>|undefined}
                 */
                public indexes?: cockroach.server.serverpb.TableDetailsResponse.Index$Properties[];

                /**
                 * @type {Long|undefined}
                 */
                public rangeCount?: Long;

                /**
                 * @type {string|undefined}
                 */
                public createTableStatement?: string;

                /**
                 * @type {cockroach.config.ZoneConfig$Properties|undefined}
                 */
                public zoneConfig?: cockroach.config.ZoneConfig$Properties;

                /**
                 * @type {cockroach.server.serverpb.ZoneConfigurationLevel|undefined}
                 */
                public zoneConfigLevel?: cockroach.server.serverpb.ZoneConfigurationLevel;

                /**
                 * @type {Long|undefined}
                 */
                public descriptorId?: Long;

                /**
                 * Creates a new TableDetailsResponse instance using the specified properties.
                 * @param {cockroach.server.serverpb.TableDetailsResponse$Properties=} [properties] Properties to set
                 * @returns {cockroach.server.serverpb.TableDetailsResponse} TableDetailsResponse instance
                 */
                public static create(properties?: cockroach.server.serverpb.TableDetailsResponse$Properties): cockroach.server.serverpb.TableDetailsResponse;

                /**
                 * Encodes the specified TableDetailsResponse message. Does not implicitly {@link cockroach.server.serverpb.TableDetailsResponse.verify|verify} messages.
                 * @param {cockroach.server.serverpb.TableDetailsResponse$Properties} message TableDetailsResponse message or plain object to encode
                 * @param {$protobuf.Writer} [writer] Writer to encode to
                 * @returns {$protobuf.Writer} Writer
                 */
                public static encode(message: cockroach.server.serverpb.TableDetailsResponse$Properties, writer?: $protobuf.Writer): $protobuf.Writer;

                /**
                 * Encodes the specified TableDetailsResponse message, length delimited. Does not implicitly {@link cockroach.server.serverpb.TableDetailsResponse.verify|verify} messages.
                 * @param {cockroach.server.serverpb.TableDetailsResponse$Properties} message TableDetailsResponse message or plain object to encode
                 * @param {$protobuf.Writer} [writer] Writer to encode to
                 * @returns {$protobuf.Writer} Writer
                 */
                public static encodeDelimited(message: cockroach.server.serverpb.TableDetailsResponse$Properties, writer?: $protobuf.Writer): $protobuf.Writer;

                /**
                 * Decodes a TableDetailsResponse message from the specified reader or buffer.
                 * @param {$protobuf.Reader|Uint8Array} reader Reader or buffer to decode from
                 * @param {number} [length] Message length if known beforehand
                 * @returns {cockroach.server.serverpb.TableDetailsResponse} TableDetailsResponse
                 * @throws {Error} If the payload is not a reader or valid buffer
                 * @throws {$protobuf.util.ProtocolError} If required fields are missing
                 */
                public static decode(reader: ($protobuf.Reader|Uint8Array), length?: number): cockroach.server.serverpb.TableDetailsResponse;

                /**
                 * Decodes a TableDetailsResponse message from the specified reader or buffer, length delimited.
                 * @param {$protobuf.Reader|Uint8Array} reader Reader or buffer to decode from
                 * @returns {cockroach.server.serverpb.TableDetailsResponse} TableDetailsResponse
                 * @throws {Error} If the payload is not a reader or valid buffer
                 * @throws {$protobuf.util.ProtocolError} If required fields are missing
                 */
                public static decodeDelimited(reader: ($protobuf.Reader|Uint8Array)): cockroach.server.serverpb.TableDetailsResponse;

                /**
                 * Verifies a TableDetailsResponse message.
                 * @param {Object.<string,*>} message Plain object to verify
                 * @returns {?string} `null` if valid, otherwise the reason why it is not
                 */
                public static verify(message: { [k: string]: any }): string;

                /**
                 * Creates a TableDetailsResponse message from a plain object. Also converts values to their respective internal types.
                 * @param {Object.<string,*>} object Plain object
                 * @returns {cockroach.server.serverpb.TableDetailsResponse} TableDetailsResponse
                 */
                public static fromObject(object: { [k: string]: any }): cockroach.server.serverpb.TableDetailsResponse;

                /**
                 * Creates a TableDetailsResponse message from a plain object. Also converts values to their respective internal types.
                 * This is an alias of {@link cockroach.server.serverpb.TableDetailsResponse.fromObject}.
                 * @function
                 * @param {Object.<string,*>} object Plain object
                 * @returns {cockroach.server.serverpb.TableDetailsResponse} TableDetailsResponse
                 */
                public static from(object: { [k: string]: any }): cockroach.server.serverpb.TableDetailsResponse;

                /**
                 * Creates a plain object from a TableDetailsResponse message. Also converts values to other types if specified.
                 * @param {cockroach.server.serverpb.TableDetailsResponse} message TableDetailsResponse
                 * @param {$protobuf.ConversionOptions} [options] Conversion options
                 * @returns {Object.<string,*>} Plain object
                 */
                public static toObject(message: cockroach.server.serverpb.TableDetailsResponse, options?: $protobuf.ConversionOptions): { [k: string]: any };

                /**
                 * Creates a plain object from this TableDetailsResponse message. Also converts values to other types if specified.
                 * @param {$protobuf.ConversionOptions} [options] Conversion options
                 * @returns {Object.<string,*>} Plain object
                 */
                public toObject(options?: $protobuf.ConversionOptions): { [k: string]: any };

                /**
                 * Converts this TableDetailsResponse to JSON.
                 * @returns {Object.<string,*>} JSON object
                 */
                public toJSON(): { [k: string]: any };
            }
@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Mar 24, 2017

The fields on the generated class should not end in "$Properties"

Let's say one .creates a message from a valid plain object incl. sub-messages, then these fields will not be runtime messages but plain objects ($Properties).

Or, if one would try to assign a plain object to these fields, tsc would complain if these were typed as runtime messages.

@tamird

This comment has been minimized.

tamird commented Mar 24, 2017

Let's say one .creates a message from a valid plain object incl. sub-messages, then these fields will not be runtime messages but plain objects ($Properties).

Oh, I didn't realize that was the behaviour. In that case, the typings are correct.

@tamird tamird closed this Mar 25, 2017

@masonk

This comment has been minimized.

masonk commented Mar 26, 2017

Not sure how I missed it before, but I think these types don't work. I am building and linking to fff1eb2 (current as of 2017-3-26 10:38 AM PST).

I think *$Properties all need to be interfaces that the message objects and constructor args all implement. All interfaces are types, but not all types are interfaces. An interface is a type that a class can implement. Classes can't implement or extend general 'types'.

src/generated/messages.d.ts(42,34): error TS2422: A class may only implement another class or interface.

I tested this kind of change to the generated definitions files and it typechecked for me.

type WebsocketEnvelope$Properties = {
    streamid?: number;
    envelope?: Envelope$Properties;
};

// change to 

interface WebsocketEnvelope$Properties {
    streamid?: number;
    envelope?: Envelope$Properties;
}

// No '=', no trailing ';'


@masonk

This comment has been minimized.

masonk commented Mar 26, 2017

I tried today - really hard - to find the code that's generate the type declaration and change it to an interface. All I can find in the patch is code that appears to add the comments for the declaration. I can't find anything that appends the code for the actual type.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Mar 26, 2017

TypeScript definitions are generated by this lib: https://github.com/dcodeIO/protobuf.js/tree/master/lib/tsd-jsdoc

There's some code for interfaces already, but I'll still have to take a look how to force whether a @typedef becomes a type or an interface solely based on JSdoc.

dcodeIO added a commit that referenced this issue Mar 26, 2017

@masonk

This comment has been minimized.

masonk commented Mar 27, 2017

In my project, this new patch produces declarations that type check.

Thanks for the tidbit about tsd-jsdoc. I was tearing my hair out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment