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

clients: Add client generation #18

Merged
merged 18 commits into from
Dec 5, 2024
Merged

clients: Add client generation #18

merged 18 commits into from
Dec 5, 2024

Conversation

febo
Copy link
Contributor

@febo febo commented Dec 2, 2024

Problem

Currently, the stake program does not include annotation to generate an IDL. This prevents using Codama to generate clients.

Solution

Use Shank to annotate the program and generate clients.

Note: Majority of source files under the clients folder are auto-generated.

@febo febo marked this pull request as ready for review December 4, 2024 17:45
Copy link

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

ezgif-1-a6c9ea68b9

clients/js/package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just the one nit.

It doesn't need to be done in this PR, but what do you think about introducing a feature like idl and gating all shank usage on that flag?

That way, users of the crate don't have to pull in shank, and even better, it should make it easier to swap to codama macros in the future with less disruptive breakage.

interface/src/instruction.rs Outdated Show resolved Hide resolved
@febo
Copy link
Contributor Author

febo commented Dec 4, 2024

Just the one nit.

It doesn't need to be done in this PR, but what do you think about introducing a feature like idl and gating all shank usage on that flag?

That way, users of the crate don't have to pull in shank, and even better, it should make it easier to swap to codama macros in the future with less disruptive breakage.

Good point! We could add shank as an optional dependency, but theoretically we don't need to add a feature for it. The annotation is only used by shank cli, so they don't do anything during compilation.

shank = { version = "0.4.2", optional = true }

This is enough to not pull in shank. The IDL generation still works without the need for a feature. Or we could add a feature, but it won't get used.

@joncinque
Copy link
Contributor

I'm not sure I see how it works without a feature (unless you mean using the implicit dep:shank feature), but I'll wait to see the code!

@febo
Copy link
Contributor Author

febo commented Dec 4, 2024

I'm not sure I see how it works without a feature (unless you mean using the implicit dep:shank feature), but I'll wait to see the code!

Hmmm, you are right. The IDL generation works without having a feature, but you will need it to be able to have the validation when you add them to your struct/enum types.

I will add the idl feature.

@febo
Copy link
Contributor Author

febo commented Dec 5, 2024

I'm not sure I see how it works without a feature (unless you mean using the implicit dep:shank feature), but I'll wait to see the code!

Hmmm, you are right. The IDL generation works without having a feature, but you will need it to be able to have the validation when you add them to your struct/enum types.

I will add the idl feature.

This did not work. Shank CLI does not "detect" the macros when their are behind a feature flag. I removed Shank macros and dependency but kept the IDL, so we can have the generated clients.

@febo febo requested a review from joncinque December 5, 2024 00:31
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Just the tiny nit, then this can go in!

interface/src/instruction.rs Outdated Show resolved Hide resolved
@febo febo merged commit e3f9f20 into master Dec 5, 2024
6 checks passed
@febo febo deleted the febo/js-client branch December 5, 2024 10:50
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.

3 participants