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

Add wasm support for both web and node environment #1

Merged
merged 14 commits into from
Oct 17, 2024
Merged

Conversation

tolak
Copy link
Collaborator

@tolak tolak commented Oct 11, 2024

This is PR is going to adding wasm support for both web and node envrionment by compile the library to wasm-unknow-unknown target.
Note that js feature should be enabled and due to tls not been supportted by WASM, get_collateral will be disabled at the time

  • Build to specific target JS packages
  • Test JS packages in both web and node env

@tolak tolak changed the title Add wasm support for both web and node envrionment Add wasm support for both web and node environment Oct 11, 2024
@0xshawn 0xshawn marked this pull request as ready for review October 15, 2024 14:35
@0xshawn 0xshawn requested a review from kvinwang October 15, 2024 14:39
Cargo.toml Outdated
Comment on lines 48 to 51
getrandom = { version = "0.2", features = ["js"] }
serde-wasm-bindgen = "0.4"
wasm-bindgen = "0.2.95"
serde_bytes = "0.11"
Copy link
Collaborator

@kvinwang kvinwang Oct 15, 2024

Choose a reason for hiding this comment

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

Let's make them optional and enable them by the feature js.

pub struct VerifiedReport {
pub status: String,
pub advisory_ids: Vec<String>,
pub report: Report,
}

#[wasm_bindgen]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#[wasm_bindgen]
#[cfg(feature = "js")]
#[wasm_bindgen]

@kvinwang
Copy link
Collaborator

Note that js feature should be enabled and due to tls not been supportted by WASM, get_collateral will be disabled at the time

AFAIK, the reqwest supports running in browser. Is there any problem to export the get_collateral as well?

Copy link
Collaborator

@kvinwang kvinwang left a comment

Choose a reason for hiding this comment

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

Great work. Thanks a lot.

@0xshawn
Copy link
Collaborator

0xshawn commented Oct 17, 2024

Here are the changes to reviewed comments:

  • Make wasm related denpendencies optional and add them to js feature
  • Fix errors reported by cargo check

@0xshawn 0xshawn merged commit 006d887 into master Oct 17, 2024
1 check passed
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