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

feat: generate typescript bindings #825

Merged
merged 12 commits into from
Oct 31, 2023
Merged

feat: generate typescript bindings #825

merged 12 commits into from
Oct 31, 2023

Conversation

fadeev
Copy link
Member

@fadeev fadeev commented Jul 19, 2023

make typescript should generate TS bindings in ./typescript.

Using https://buf.build/blog/protobuf-es-the-protocol-buffers-typescript-javascript-runtime-we-all-deserve

We probably don't need TS bindings in the repo. This should be generated automatically on release and published on npm.

@lumtis
Copy link
Member

lumtis commented Jul 30, 2023

Just wondering, why is it in draft?

@fadeev
Copy link
Member Author

fadeev commented Jul 30, 2023

Just wondering, why is it in draft?

I want the fullstack team to check it out and try using to see if we have everything in place before merging.

@GMaiolo
Copy link

GMaiolo commented Aug 9, 2023

Could we remove the _pb suffix for prolixity? I feel it makes sense for consumers; unless it causes an issue within the node repository.


Is it possible to have an index.d.ts file that exports everything within its folder? e.g.:

crosschain/
  index.d.ts
  foo.d.ts
  bar.d.ts
  baz.d.ts
  

With index.d.ts being something like the following:

export * from "./foo"
export * from "./bar"
export * from "./baz"

This way, we can consume types directly from the known folder (crosschain) and we don't need to know the specific file name (foo, bar, baz).

@fadeev
Copy link
Member Author

fadeev commented Aug 11, 2023

@GMaiolo done! ✅

@fadeev fadeev marked this pull request as ready for review August 11, 2023 09:27
@fadeev
Copy link
Member Author

fadeev commented Aug 11, 2023

@lucas-janon can you, please, add an npm token, so we can set it up to publish automatically when a release is created.

@GMaiolo
Copy link

GMaiolo commented Aug 11, 2023

@fadeev could we manually publish a 0.0.1 version in NPM so I can test the usage?

@fadeev
Copy link
Member Author

fadeev commented Aug 12, 2023

I can't publish, because I don't have an npm token. Have you tried with yarn link first?

@fadeev fadeev marked this pull request as draft October 3, 2023 17:14
@lumtis
Copy link
Member

lumtis commented Oct 27, 2023

Can we put as ready for review @fadeev ?

@fadeev fadeev marked this pull request as ready for review October 28, 2023 12:53
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you run make typescript with the latest changes?
I get several modified files when I run the command

Makefile Show resolved Hide resolved
@fadeev
Copy link
Member Author

fadeev commented Oct 30, 2023

@GMaiolo can you, please, review the code generated in this PR and let us know if this matches your requirements? Thanks!

Copy link

@GMaiolo GMaiolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lumtis lumtis merged commit c324935 into develop Oct 31, 2023
13 checks passed
@lumtis lumtis deleted the feat/gen-ts branch October 31, 2023 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants