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.

Sign up for GitHub

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

Jump to bottom

Expose policy info and utility methods to be available in the web-client #1835

Merged
merged 6 commits into from
Sep 11, 2023
Merged

Expose policy info and utility methods to be available in the web-client #1835

merged 6 commits into from
Sep 11, 2023

Conversation

Eligioo
Copy link
Member

@Eligioo Eligioo commented Sep 8, 2023

What's in this pull request?

Annotate Policy primitives including the static utility functions with wasm-bindgen so they can be used by the web-client.

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@Eligioo Eligioo requested review from sisou and jsdanielh September 8, 2023 14:25
sisou
sisou requested changes Sep 8, 2023
View reviewed changes
primitives/src/policy.rs Outdated Show resolved Hide resolved
primitives/src/policy.rs Outdated Show resolved Hide resolved
@sisou
Copy link
Member

sisou commented Sep 9, 2023

I fixed a few things, now the only thing remaining is to add getters for the pub consts when the "ts-types" feature is enabled.

@sisou
Copy link
Member

sisou commented Sep 9, 2023 •
edited
Loading

Added the getters for the consts. However, now we have two types of getters. CAPITAL_SNAKE_CASED and pascalCased ones:

image

I am torn between:

  1. Changing the pascalCased ones to CAPITAL_SNAKE_CASE, because that would be consistent with best-practices in JS/TS for static, never-changing (during runtime) variables. This renaming would only affect the JS-glue code, no renaming in Rust is required.
  2. Keeping them as-is, to keep them consistent with how they are defined in Rust.

Any opinions?

jsdanielh
jsdanielh reviewed Sep 11, 2023
View reviewed changes
primitives/src/policy.rs Outdated Show resolved Hide resolved
Eligioo and others added 5 commits September 11, 2023 16:34
@Eligioo @sisou
Expose policy info and utility methods to be available in the web-client
f008273
@sisou
Conditionally import wasm_bindgen
7d0a206
@sisou
Add WASM getter definitions for Policy getters
757e7e8
@sisou
Skip Policy's own fields in WASM, remove Client.policy()
d56c41e 
The struct's own fields don't work without initialization. They are replaced by the getters from the previous commit, that do initialization internally.

Thanks to @hrxi for finding the `cfg_eval` workaround to use wasm_bindgen attributes on struct fields.
@sisou
Reimplement Policy's pub consts as getters for WASM
70eacbe
@sisou sisou force-pushed the stefan/web-client-policy branch from 29ebe55 to 70eacbe Compare September 11, 2023 14:41
@jsdanielh jsdanielh merged commit 147bdc8 into albatross Sep 11, 2023
@jsdanielh jsdanielh deleted the stefan/web-client-policy branch September 11, 2023 15:37
@jsdanielh jsdanielh added this to the Nimiq PoS Mainnet milestone Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sisou sisou sisou approved these changes

@jsdanielh jsdanielh jsdanielh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Nimiq PoS Mainnet
Development

Successfully merging this pull request may close these issues.
3 participants
@Eligioo @sisou @jsdanielh