protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

Missing interfaces in typescript file ~probably is jsdoc issue~ [It is CLI issue]

Open VitalyName opened this issue 4 years ago • 16 comments

protobuf.js version: 6.9.0 nodejs: 14.2.0

If I have class after enum in JS file (after pbjs command), I can't see interfaces for it in TS file after pbts command. But interfaces for classes before enum is in TS file and they work fine. However if I change jsdoc from 3.6.4 to 3.5.5 version, all interface is generated fine.

Examle:

/**
 * IntSize enum.
 * @exports IntSize
 * @enum {number}
 * @property {number} IS_DEFAULT=0 IS_DEFAULT value
 * @property {number} IS_8=8 IS_8 value
 * @property {number} IS_16=16 IS_16 value
 * @property {number} IS_32=32 IS_32 value
 * @property {number} IS_64=64 IS_64 value
 */
$root.IntSize = (function() {
    var valuesById = {}, values = Object.create(valuesById);
    values[valuesById[0] = "IS_DEFAULT"] = 0;
    values[valuesById[8] = "IS_8"] = 8;
    values[valuesById[16] = "IS_16"] = 16;
    values[valuesById[32] = "IS_32"] = 32;
    values[valuesById[64] = "IS_64"] = 64;
    return values;
})();

$root.NanoPBOptions = (function() {

    /**
     * Properties of a NanoPBOptions.
     * @exports INanoPBOptions
     * @interface INanoPBOptions
     * @property {number|null} [maxSize] NanoPBOptions maxSize
     * @property {number|null} [maxLength] NanoPBOptions maxLength
     * @property {number|null} [maxCount] NanoPBOptions maxCount
     * @property {IntSize|null} [intSize] NanoPBOptions intSize
     * @property {FieldType|null} [type] NanoPBOptions type
     * @property {boolean|null} [longNames] NanoPBOptions longNames
     * @property {boolean|null} [packedStruct] NanoPBOptions packedStruct
     * @property {boolean|null} [packedEnum] NanoPBOptions packedEnum
     * @property {boolean|null} [skipMessage] NanoPBOptions skipMessage
     * @property {boolean|null} [noUnions] NanoPBOptions noUnions
     * @property {number|null} [msgid] NanoPBOptions msgid
     * @property {boolean|null} [anonymousOneof] NanoPBOptions anonymousOneof
     * @property {boolean|null} [proto3] NanoPBOptions proto3
     * @property {boolean|null} [enumToString] NanoPBOptions enumToString
     * @property {boolean|null} [fixedLength] NanoPBOptions fixedLength
     * @property {boolean|null} [fixedCount] NanoPBOptions fixedCount
     */

I debug this. I seen list of Doclet objects in taffy().get() here: my_project/node_modules/protobufjs/cli/lib/tsd-jsdoc/publish.js

// JSDoc hook
exports.publish = function publish(taffy, opts) {
    options = opts || {};

Many Doclet objects have field "memberof". Next in source code it field used for filtering of element list.

Whre I use jsdoc 3.5.5, Doclet for interface INanoPBOptions is memberof "undefined". So it is root element and interface will be created in TS. Whre I use jsdoc 3.6.4, Doclet for interface INanoPBOptions is memberof "module:IntSize". It take it from enum? So it is not root element and interface will not be created in TS.

VitalyName avatar May 28 '20 11:05 VitalyName

I am able to recreate this issue with my project.

  • Node version: 12.13.1
  • protobufjs: 6.9.0

Here's a short proto file to recreate:

syntax = "proto3";

message GoodMessage {
  uint32 test_1 = 1;
}

enum BreakingEnum {
  VALUE_1 = 0;
  VALUE_2 = 1;
}

message BrokenMessage {
  uint32 test_2 = 1;
}

Commands I ran to recreate: yarn pbjs -t static-module -w commonjs -o test.js test.proto yarn pbts -o test.d.ts test.js

In test.d.ts there is no definition for IBrokenMessage interface which the class BrokenMessage implements.

My current workaround is to move all file level enums to the bottom of my proto file.

ejohnso49 avatar Aug 15 '20 17:08 ejohnso49

Thanks for this (ugly) workaround!

p0wl avatar Sep 25 '20 13:09 p0wl

I have found temporary solution how to fixed it. Tested on: protobuf.js 6.10.2, nodejs 12.14.1 and nodejs 14.15.1 too.

Problem what I have solved:

  1. pbts skips creating interfaces if jsdoc version in his dependencies is above 3.5.5. (See top of this topic).
  2. If version of jsdoc is 3.5.5, then it works fine with older nodes, but then it does not work with nodejs 12 and 14.

Solution:

  1. I forked jsdoc from 3.5.5 and add only one fix what fix problem with nodejs 12 and 14 compatibility. https://github.com/VitalyName/jsdoc/commit/80443ca629052fc7aa8ff83ad3331be3123e39ce
  2. I forked protobuf.js and change source for jsdoc in his dependencies to commit from step one. https://github.com/VitalyName/protobuf.js/commit/236a7c757a5eada7926fdff149d0b49ada838275
  3. I added to package.json in my project new dev dependecies (commit from step 2)
  "devDependencies": {
    "protobufjs": "git://github.com/VitalyName/protobuf.js.git#236a7c757a5eada7926fdff149d0b49ada838275"
  }

instead of

  "devDependencies": {
    "protobufjs": "6.10.2"
  }

This is obviously, you need to use only step 3 to fix it

VitalyName avatar Jan 29 '21 13:01 VitalyName

@VitalyName do you know if protobufjs already includes a fix like yours ? Seems like holding on the the forked version is not working with node 16 anymore:

npm ERR! Cannot read properties of null (reading 'pickAlgorithm')

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\denib\AppData\Local\npm-cache\_logs\2022-09-20T07_45_13_723Z-debug-0.log

deni-begaj avatar Sep 20 '22 07:09 deni-begaj

It seems like with jsdom 3.5.5 enums are constants. Not modules. https://github.com/protobufjs/protobuf.js/blob/d0268490cb08d2f88e0aafd1d0eb673bd2d3c714/cli/lib/tsd-jsdoc/publish.js#L438

jolting avatar Oct 11 '22 18:10 jolting

Probably a bug in the jsdoc. memberof gets set by the enum.

"memberof": "module:<ENUM>",

There is a check in getChildrenOf

        return element.memberof === memberof;

It's no longer undefined.

jolting avatar Oct 11 '22 19:10 jolting

Possible fix? https://github.com/jsdoc/jsdoc/pull/1686

jolting avatar Oct 11 '22 19:10 jolting

Dupe? #1222

jolting avatar Oct 11 '22 19:10 jolting

@VitalyName it looks like protobuf.js is using the @exports tag incorrectly.

https://jsdoc.app/tags-exports.html https://jsdoc.app/howto-es2015-modules.html

There should be only one per module and it really shouldn't be used with es6 style module exports. I'm generating a static bundle like this. pbjs -t static-module -w es6 -o ./dist/bundle.js ../proto3/*.proto

Every exported class has a @exports tag. It seems like jsdoc 3.5.5 was just allowing protobuf.js to get away with the error.

jolting avatar Oct 12 '22 00:10 jolting

@VitalyName The best workaround I've come up with is simply filter the bundle.js file for for /^ *\* @exports/ lines, given these are being used incorrectly in protobufjs. This should fix your problem.

jolting avatar Oct 12 '22 17:10 jolting

@VitalyName it looks like protobuf.js is using the @exports tag incorrectly.

https://jsdoc.app/tags-exports.html https://jsdoc.app/howto-es2015-modules.html

There should be only one per module and it really shouldn't be used with es6 style module exports. I'm generating a static bundle like this. pbjs -t static-module -w es6 -o ./dist/bundle.js ../proto3/*.proto

Every exported class has a @exports tag. It seems like jsdoc 3.5.5 was just allowing protobuf.js to get away with the error.

Thank you! It is really works and it solves my issue. PS: I have edited this message. My bad and it was my foult to build it. Now With your fix all is working well.

VitalyName avatar Oct 25 '22 13:10 VitalyName

I'm glad it worked. I finally settled on rewriting all the @exports to @name. Currently I'm not depending on the PR. Although I would hope a fix of this fashion gets merged.

I was thinking about revising the PR to fix all the exports. For commonjs put the @exports on the root. I haven't finished that yet.

Right now I'm processing the file like this prior to pbts.

  for await (const line of rl) {
    const regex = /^( *\* )@exports/;
    outputFile.write(line.replace(regex, `$1@name`));
    outputFile.write('\n');
  }

In my project, commonjs and es6 generate the same TS file, so I don't think there is a problem.

jolting avatar Oct 25 '22 15:10 jolting

Right now I'm processing the file like this prior to pbts.

Sorry My bad. Commonjs works fine too. I have edited my previous message.

VitalyName avatar Oct 25 '22 15:10 VitalyName

@jolting your workaround worked for me. Now I can use latest protobufjs, while not having TS compile error 😄 Thanks a lot.

deni-begaj avatar Dec 21 '22 22:12 deni-begaj

I have doubled it here. Because solution was missed of many peoples who read this topic. Also this issue does not go away with latest protobuffjs 7.2.0 cli 1.1.0 update which is related with migration to jsdoc4. Clear solution by @jolting is here https://github.com/protobufjs/protobuf.js/pull/1824

But if you don't want to wait for it to be merged, you can use workaround a few posts above

VitalyName avatar Jan 27 '23 06:01 VitalyName

https://github.com/protobufjs/protobuf.js/pull/1999

jaovitubr avatar Jun 02 '24 17:06 jaovitubr