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

Any good way to generate TypeScript definition for static .js files from pbjs ? #550

Closed
k8w opened this Issue Dec 13, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@k8w

k8w commented Dec 13, 2016

See that protobuf.js generate .d.ts via jsdoc, it is useful for dynamic use case.
While for static code generate, any good way to generate this ?

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 13, 2016

This is planned and will probably be available soon. In the meantime you can look at how protobuf.js generates the .d.ts and apply that to the statically generated files.

@dcodeIO dcodeIO added the enhancement label Dec 13, 2016

dcodeIO added a commit that referenced this issue Dec 13, 2016

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 13, 2016

Give it a try! (use --name instead of -n for now)

@jeffgrossmann

This comment has been minimized.

jeffgrossmann commented Dec 13, 2016

First, thanks for adding this feature. I installed the latest version to test out pbts and it works if the *.proto files do NOT include a package statement. With a package statement, the generated d.ts only contains single var with the package name. See attached proto files and generated output in the zip.
pbts_example.zip

nopackage.proto: Does not have a package and generates the correct nopackage.d.ts
simple.proto: Has a package statement and generates an incomplete simple.d.ts

Test steps

  1. run pbjs to generate js from proto
  2. run pbts to generate d.ts from js

Thanks much.

dcodeIO added a commit that referenced this issue Dec 13, 2016

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 13, 2016

Does this fix it?

@jeffgrossmann

This comment has been minimized.

jeffgrossmann commented Dec 13, 2016

Yes, the generated d.ts looks good. I need to test with some typescript code to verify the types but visual inspection is a "pass".

Thanks again.

@k8w

This comment has been minimized.

k8w commented Dec 14, 2016

Must use absolute path ?

This get error
pbjs -n test test.js

ERROR: Unable to find the source file or directory C:\Users\longsheng\AppData\Roaming\npm\node_modules\protobufjs\test.js

and this works
pbjs -n test E:\xxx\xxx\xxx\test.js

@osechet

This comment has been minimized.

osechet commented Dec 14, 2016

Nested classes are not generated properly with pbts. In typescript definition, nested classes must be added to a module whose name is the same as the parent class:

message A {
  message B {
  }
}

should give:

class A {
}
module A {
  class B {
  }
}
@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 14, 2016

@twoeo: Did you mean pbts?

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 14, 2016

@osechet: That doesn't appear correct to me. Or is it? The dts of the library itself doesn't use something like this and validates properly against latest dev typescript

@osechet

This comment has been minimized.

osechet commented Dec 14, 2016

@dcodeIO The concept of nested class does not exist in Typescript. The only way to make something equivalent is the way I described. You can find a lot of resources on Internet that explains it, i.e:
http://stackoverflow.com/questions/13495107/any-way-to-declare-a-nest-class-structure-in-typescript
https://blog.oio.de/2014/03/21/inner-classes-typescript/

But I think it's a problem with tsd-jsdoc. I remember running into a similar problem last time I used it.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 14, 2016

Can you provide me with an example output that fails?

dcodeIO added a commit that referenced this issue Dec 14, 2016

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 14, 2016

@twoeo: This commit should fix the absolute path issue you described. Also make sure that the tsd-jsdoc dependency is updated to current master (there has been an issue with piped output).

dcodeIO added a commit that referenced this issue Dec 15, 2016

Post-merge; Repo now includes a restructured version of tsd-jsdoc wit…
…h our changes incorporated for issues/prs, see #550
@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 15, 2016

I've restructured tsd-jsdoc and incorporated our changes, which is now part of this repository. There are a few more comments on what the different functions actually do making the code relatively easy to understand. Hence, it's now possible to process issues and PRs.

@dcodeIO

This comment has been minimized.

Owner

dcodeIO commented Dec 15, 2016

As tsd-jsdoc puts inner classes into modules now and all other issues mentioned seem to be resolved, I am closing this for now. Feel free to create new issues or PRs referencing lib/jsdoc directly.

@dcodeIO dcodeIO closed this Dec 15, 2016

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