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

Add WASI example for CI. #411

Merged
merged 8 commits into from
Dec 6, 2023
Merged

Add WASI example for CI. #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
775f3aa
feat: Support WASI target.
langyo Nov 20, 2023
43893b5
fix: More friendly error message for wasi target.
langyo Nov 20, 2023
9f6d39f
fix: Use target_os instead of feature.
langyo Nov 30, 2023
2676fe6
feat: Add an example for gloo-history on WASI.
langyo Nov 30, 2023
a3fae2c
fix: Avoid to use web-sys when target os is WASI.
langyo Nov 30, 2023
b35492f
fix: Exclude WASI example to avoid web-sys.
langyo Dec 5, 2023
d5cad9a
ci: Use static download instead of install bash.
langyo Dec 5, 2023
4a91317
ci: Use CLI's flag instead of exclude example.
langyo Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 46 additions & 9 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: Tests
on:
pull_request:
push:
branches: [ master ]
branches: [master]

jobs:
native_tests:
Expand Down Expand Up @@ -119,14 +119,52 @@ jobs:
wasm-pack test --node crates/$x --no-default-features
done

test-history-wasi:
name: Test gloo-history WASI
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
rust-version: [1.64, stable, nightly]
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust-version }}
target: wasm32-wasi

- name: Install wasmtime
run: |
wget https://github.com/bytecodealliance/wasmtime/releases/download/v15.0.1/wasmtime-v15.0.1-x86_64-linux.tar.xz
tar xf wasmtime-v15.0.1-x86_64-linux.tar.xz
mv wasmtime-v15.0.1-x86_64-linux/wasmtime ~/wasmtime
rm -rf wasmtime-v15.0.1-x86_64-linux.tar.xz wasmtime-v15.0.1-x86_64-linux
chmod +x ~/wasmtime

- uses: actions/cache@v3
with:
path: |
~/.cargo/registry
~/.cargo/git
target
key: cargo-${{ runner.os }}-node-tests-${{ hashFiles('**/Cargo.toml') }}
restore-keys: |
cargo-${{ runner.os }}-node-tests-
cargo-${{ runner.os }}-

- name: Build and run example history-wasi
run: |
cargo build --package example-history-wasi --target wasm32-wasi
~/wasmtime --trap-unknown-imports target/wasm32-wasi/debug/example-history-wasi.wasm

test-worker:
name: Test gloo-worker
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
# example: [ markdown, prime ]
example: [ markdown ]
example: [markdown]
rust-version: [1.64, stable, nightly]
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -167,7 +205,6 @@ jobs:
run: |
wasm-pack test --headless --chrome --firefox examples/${{ matrix.example }}


test-net:
strategy:
fail-fast: false
Expand Down Expand Up @@ -208,9 +245,9 @@ jobs:

- name: Run browser tests
env:
HTTPBIN_URL: "http://localhost:8080"
WS_ECHO_SERVER_URL: "ws://localhost:8081"
SSE_ECHO_SERVER_URL: "http://localhost:8081/.sse"
HTTPBIN_URL: 'http://localhost:8080'
WS_ECHO_SERVER_URL: 'ws://localhost:8081'
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: |
cd crates/net
wasm-pack test --chrome --firefox --headless --all-features
Expand All @@ -222,7 +259,7 @@ jobs:

- name: Run native tests
env:
HTTPBIN_URL: "http://localhost:8080"
WS_ECHO_SERVER_URL: "ws://localhost:8081"
SSE_ECHO_SERVER_URL: "http://localhost:8081/.sse"
HTTPBIN_URL: 'http://localhost:8080'
WS_ECHO_SERVER_URL: 'ws://localhost:8081'
SSE_ECHO_SERVER_URL: 'http://localhost:8081/.sse'
run: cargo test -p gloo-net --all-features
11 changes: 9 additions & 2 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ members = [
"examples/markdown",
"examples/clock",
"examples/file-hash",
"examples/history-wasi",
"examples/prime",
]

Expand Down
6 changes: 4 additions & 2 deletions crates/history/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ categories = ["api-bindings", "history", "wasm"]
rust-version = "1.64"

[dependencies]
wasm-bindgen = "0.2"
gloo-utils = { version = "0.2.0", path = "../utils" }
gloo-events = { version = "0.2.0", path = "../events" }
serde = { version = "1", features = ["derive"] }
serde-wasm-bindgen = "0.6.0"
serde_urlencoded = { version = "0.7", optional = true }
thiserror = { version = "1.0", optional = true }

[dependencies.web-sys]
[target.'cfg(not(target_os = "wasi"))'.dependencies]
wasm-bindgen = "0.2"

[target.'cfg(not(target_os = "wasi"))'.dependencies.web-sys]
version = "0.3"
features = ["History", "Window", "Location", "Url"]

Expand Down
30 changes: 30 additions & 0 deletions crates/history/src/any.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::borrow::Cow;

#[cfg(not(target_os = "wasi"))]
use crate::browser::BrowserHistory;
#[cfg(not(target_os = "wasi"))]
use crate::hash::HashHistory;
use crate::history::History;
use crate::listener::HistoryListener;
Expand All @@ -13,8 +15,10 @@ use crate::{error::HistoryResult, query::ToQuery};
#[derive(Clone, PartialEq, Debug)]
pub enum AnyHistory {
/// A Browser History.
#[cfg(not(target_os = "wasi"))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are the effects of enabling BrowserHistory and MemoryHistory for wasi?

I think wasi should have BrowserHistory and MemoryHistory enabled like other targets.
This helps to avoid additional feature flags for downstream crates and will lint the code like other targets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What are the effects of enabling BrowserHistory and MemoryHistory for wasi?

I think wasi should have BrowserHistory and MemoryHistory enabled like other targets. This helps to avoid additional feature flags for downstream crates and will lint the code like other targets.

If don't, BrowserHistory is directly dependent on wasm-bindgen. It will cause additional symbols like __wbindgen_placeholder__::xxx to appear in the compiled result WASM, which is invalid in WASI.

These changes that I've made to some other repositories recently were also made to fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think WASI has been disabled in rustwasm/wasm-bindgen#3233?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think WASI has been disabled in rustwasm/wasm-bindgen#3233?

It seems like it will still try to import symbols if you actively reference it. This is a bit different from the situation where wasm-bindgen passively imports utility functions related to heap allocation.

Personally, I feel that it is not a bad thing to proactively limit the compilation targets available to the code. Because this allows the compiler to warn developers faster.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems like it will still try to import symbols if you actively reference it.

I think you may need to run cargo update. Since this is only released in 0.2.88 last month.

I tried the following code:

fn main() {
    js_sys::Date::now();
}
image

Personally, I feel that it is not a bad thing to proactively limit the compilation targets available to the code. Because this allows the compiler to warn developers faster.

This is a design decision made all the way up to wasm-bindgen.
One of the reasons is that you can write code targeting both wasm (client) and native targets (server) at the same time without having to reconfigure toolchain.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@langyo Since 0.2.88 solves the WASI issues, would it be possible to create a pull request to remove the feature flags from gloo-history so we do not subsequently introduce more feature flags into Yew?

Copy link
Contributor Author

@langyo langyo Dec 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@langyo Since 0.2.88 solves the WASI issues, would it be possible to create a pull request to remove the feature flags from gloo-history so we do not subsequently introduce more feature flags into Yew?

I think it's better not to remove it yet. Keeping some flags will help users who compile with WASI to quickly locate the cause of the error.

But I will actively consider this matter... Perhaps there's no need to resort to strong measures to circumvent this problem. 🤔

Copy link
Collaborator

@futursolo futursolo Dec 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Keeping some flags will help users who compile with WASI to quickly locate the cause of the error.

I think it is important to ensure consistency between gloo and wasm-bindgen handling.

When users wrongly uses gloo-history under wasi with client side rendering code, they pretty much uses wasm-bindgen and web-sys wrongly as well. So the user will inevitably be facing runtime errors even if gloo-history uses compiler flags.

I have tried to apply a similar approach with target flags to make Yew Send, and it resulted in a very large number of feature flags being used and it will require every user who writes SSR to apply a large number of feature flags.
I have reached a conclusion that using feature flags as unfeasible for end users.

The current approach of wasm-bindgen, gloo and Yew allow users to avoid compiler flags in most situations whlist still be compatible with both browser and native targets.

In addition, it will eventually be possible for wasm32-wasi to run under browser environments.
These feature flags will need to be removed to provide that support, which will be another breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your opinion is also very reasonable. I thought about it for a while and I'll try to roll back a part of the code later.

On the possibility of WASI running in the browser. Personally, I don't think it would be appropriate for WASI to be a well-designed virtual machine interface that would be arbitrarily introduced as a non-standard interface. It's not for technical reasons, but because of some historical lessons... If some non-standard interfaces are adopted on a large scale, it will be very troublesome to standardize for them in the future.

As a recent example, wasmer has actually made an interface that is slightly different from standard WASI. Even though this made it become the first WASM runtime written in Rust to be able to communicate over the network, it came at the cost of introducing a large number of specialized patch libraries, even including the Rust compiler.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On the possibility of WASI running in the browser.

Browsers are already on track to support WASI.
What is not yet decided is that whether and how wasm-bindgen wants to support wasi.

https://bugzilla.mozilla.org/show_bug.cgi?id=1701197

Browser(BrowserHistory),
/// A Hash History
#[cfg(not(target_os = "wasi"))]
Hash(HashHistory),
/// A Memory History
Memory(MemoryHistory),
Expand All @@ -23,31 +27,39 @@ pub enum AnyHistory {
impl History for AnyHistory {
fn len(&self) -> usize {
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.len(),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.len(),
Self::Memory(m) => m.len(),
}
}

fn go(&self, delta: isize) {
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.go(delta),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.go(delta),
Self::Memory(m) => m.go(delta),
}
}

fn push<'a>(&self, route: impl Into<Cow<'a, str>>) {
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.push(route),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.push(route),
Self::Memory(m) => m.push(route),
}
}

fn replace<'a>(&self, route: impl Into<Cow<'a, str>>) {
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.replace(route),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.replace(route),
Self::Memory(m) => m.replace(route),
}
Expand All @@ -58,7 +70,9 @@ impl History for AnyHistory {
T: 'static,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.push_with_state(route, state),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.push_with_state(route, state),
Self::Memory(m) => m.push_with_state(route, state),
}
Expand All @@ -69,7 +83,9 @@ impl History for AnyHistory {
T: 'static,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.replace_with_state(route, state),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.replace_with_state(route, state),
Self::Memory(m) => m.replace_with_state(route, state),
}
Expand All @@ -85,7 +101,9 @@ impl History for AnyHistory {
Q: ToQuery,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.push_with_query(route, query),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.push_with_query(route, query),
Self::Memory(m) => m.push_with_query(route, query),
}
Expand All @@ -100,7 +118,9 @@ impl History for AnyHistory {
Q: ToQuery,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.replace_with_query(route, query),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.replace_with_query(route, query),
Self::Memory(m) => m.replace_with_query(route, query),
}
Expand All @@ -118,7 +138,9 @@ impl History for AnyHistory {
T: 'static,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.push_with_query_and_state(route, query, state),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.push_with_query_and_state(route, query, state),
Self::Memory(m) => m.push_with_query_and_state(route, query, state),
}
Expand All @@ -136,7 +158,9 @@ impl History for AnyHistory {
T: 'static,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.replace_with_query_and_state(route, query, state),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.replace_with_query_and_state(route, query, state),
Self::Memory(m) => m.replace_with_query_and_state(route, query, state),
}
Expand All @@ -147,27 +171,33 @@ impl History for AnyHistory {
CB: Fn() + 'static,
{
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.listen(callback),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.listen(callback),
Self::Memory(m) => m.listen(callback),
}
}

fn location(&self) -> Location {
match self {
#[cfg(not(target_os = "wasi"))]
Self::Browser(m) => m.location(),
#[cfg(not(target_os = "wasi"))]
Self::Hash(m) => m.location(),
Self::Memory(m) => m.location(),
}
}
}

#[cfg(not(target_os = "wasi"))]
impl From<BrowserHistory> for AnyHistory {
fn from(m: BrowserHistory) -> AnyHistory {
AnyHistory::Browser(m)
}
}

#[cfg(not(target_os = "wasi"))]
impl From<HashHistory> for AnyHistory {
fn from(m: HashHistory) -> AnyHistory {
AnyHistory::Hash(m)
Expand Down
4 changes: 4 additions & 0 deletions crates/history/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
#![deny(missing_docs, missing_debug_implementations)]

mod any;
#[cfg(not(target_os = "wasi"))]
mod browser;
#[cfg(feature = "query")]
mod error;
#[cfg(not(target_os = "wasi"))]
mod hash;
mod history;
mod listener;
Expand All @@ -18,7 +20,9 @@ mod state;
mod utils;

pub use any::AnyHistory;
#[cfg(not(target_os = "wasi"))]
pub use browser::BrowserHistory;
#[cfg(not(target_os = "wasi"))]
pub use hash::HashHistory;
pub use memory::MemoryHistory;

Expand Down
2 changes: 2 additions & 0 deletions crates/history/src/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg(not(target_os = "wasi"))]

use std::any::Any;
use std::collections::HashMap;
use std::rc::Rc;
Expand Down
14 changes: 12 additions & 2 deletions crates/history/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@ use std::cell::RefCell;
use std::rc::{Rc, Weak};
use std::sync::atomic::{AtomicU32, Ordering};

#[cfg(not(target_os = "wasi"))]
use wasm_bindgen::throw_str;

#[cfg(not(target_arch = "wasm32"))]
#[cfg(any(not(target_arch = "wasm32"), target_os = "wasi"))]
pub(crate) fn get_id() -> u32 {
static ID_CTR: AtomicU32 = AtomicU32::new(0);

ID_CTR.fetch_add(1, Ordering::SeqCst)
}

#[cfg(target_arch = "wasm32")]
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
pub(crate) fn get_id() -> u32 {
static ID_CTR: AtomicU32 = AtomicU32::new(0);
static INIT: std::sync::Once = std::sync::Once::new();
Expand All @@ -30,19 +31,28 @@ pub(crate) fn get_id() -> u32 {

pub(crate) fn assert_absolute_path(path: &str) {
if !path.starts_with('/') {
#[cfg(not(target_os = "wasi"))]
throw_str("You cannot use relative path with this history type.");
#[cfg(target_os = "wasi")]
panic!("You cannot use relative path with this history type.");
}
}

pub(crate) fn assert_no_query(path: &str) {
if path.contains('?') {
#[cfg(not(target_os = "wasi"))]
throw_str("You cannot have query in path, try use a variant of this method with `_query`.");
#[cfg(target_os = "wasi")]
panic!("You cannot have query in path, try use a variant of this method with `_query`.");
}
}

pub(crate) fn assert_no_fragment(path: &str) {
if path.contains('#') {
#[cfg(not(target_os = "wasi"))]
throw_str("You cannot use fragments (hash) in memory history.");
#[cfg(target_os = "wasi")]
panic!("You cannot use fragments (hash) in memory history.");
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/history/tests/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![cfg(feature = "query")]

#[cfg(target_arch = "wasm32")]
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
use wasm_bindgen_test::{wasm_bindgen_test as test, wasm_bindgen_test_configure};
#[cfg(target_arch = "wasm32")]
#[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))]
wasm_bindgen_test_configure!(run_in_browser);

use gloo_history::query::*;
Expand Down
Loading