-
Notifications
You must be signed in to change notification settings - Fork 6
Add wasm32 target conditional compilation support #66
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Ryan Tate <[email protected]>
405309f to
1be9740
Compare
| Ok(Self { key, jwk }) | ||
| } | ||
|
|
||
| pub fn from_pkcs8_pem(s: &str) -> Result<Self> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I don't think this should be added to the crate. It means we make the pkcs8 feature of p256 mandatory, when a dependent might just want to use DER
| # There may be a better way to handle this that doesn't require the `json-syntax` crate directly. | ||
| json-syntax = { version = "0.12.5", features = ["serde_json"] } | ||
| jsonschema = "0.18.0" | ||
| jsonschema = { git = "https://github.com/spruceid/jsonschema.git", branch = "feat/wasm-support" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you planning on raising a PR with the changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, yes, but receiving a Pull request creation failed. Validation failed: must be a collaborator error when opening a PR against the origin repository. We can merge this branch into our forked main branch, but likely will need to reach out to the repo owner and allow him to accept changes upstream.
Cargo.toml
Outdated
| serde_urlencoded = "0.7.1" | ||
| ssi = { version = "0.12", features = ["secp256r1"] } | ||
| tokio = "1.32.0" | ||
| tokio = { version = "1.43.0", features = ["sync"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this feature needed?
| pub struct CompiledJsonSchema { | ||
| raw: serde_json::Value, | ||
| compiled: Arc<jsonschema::JSONSchema>, | ||
| compiled: Arc<jsonschema::Resource>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this Arc still needed if the methods now don't return self.compiled directly?
src/core/util/mod.rs
Outdated
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct ReqwestClient(reqwest::Client); | ||
| #[cfg(not(target_arch = "wasm32"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it mean that the "client" code isn't fully available on WASM? Why not use reqwest's default WASM client (https://docs.rs/reqwest/latest/reqwest/#wasm) and simply disable rusttls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were some tls and cookie related features that get disabled with the reqwest client when the target is automatically set. I split those those methods into conditional compilations, and we are able to have a working client for a wasm target rather than target gating the entire client code.
Signed-off-by: Ryan Tate <[email protected]>
d3ffbae to
8d7ccc5
Compare
This pull request introduces updates to enable WASM compatibility. The most important changes include updating dependencies (notably
jsonschema,serde,tokio, andreqwest), refactoring for WASM support (including conditional compilation and trait adjustments), and improving the handling and validation of JSON schemas.Dependency Updates and WASM Compatibility:
Cargo.tomlto use a WASM-compatible fork ofjsonschema, newer versions ofserde,tokio(with appropriate features), and separatedreqwestdependencies for WASM and non-WASM targets. [1] [2]async_traitattributes for WASM compatibility, including inClient,RequestSigner, andSessionStoretraits. [1] [2] [3] [4] [5] [6]JSON Schema Handling Improvements:
jsonschema::JSONSchematojsonschema::Resourcegiven interface changes, and updated related methods to use the new validator construction pattern. [1] [2] [3] [4]Session and Verifier Refactoring:
Verifierstruct fields public and adjustedSessionStoretrait and builder patterns to support both WASM and non-WASM targets, improving flexibility and testability. [1] [2] [3]Session. [1] [2] [3] [4]HTTP Client Abstraction:
Cryptographic Enhancements and Testing:
P256Signerfor constructing from PKCS#8 PEM, and included a corresponding test to verify its functionality. [1] [2]Test Suite Adjustments: