Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate #4122

zzorba · 2024-10-12T18:33:08Z

This change consists mostly of type-wrangling to support building the matrix-sdk-ui crate for wasm32-unknown-unknown. It is a step along the way of (ideally) supporting wasm32 throughout the entire project. The matrix-sdk-ui packages is not yet compiling due to some blocking issues, but these changes should be able to be landed now as a step towards that goal.

The changes consist of one of two main types.

Promoting some types related to futures to the matrix-sdk-base, where an existing 'polyfill' of sorts for Futures already existed. This includes stuff around 'boxing' futures and aborting them.
Switching to use the matrix-sdk-base spawn instead of tokio::spawn. On non-wasm32 platforms this should be a noop.

Public API changes documented in changelogs (optional)

Signed-off-by: Daniel Salinas

crates/matrix-sdk-base/src/event_cache_store/traits.rs

crates/matrix-sdk-base/src/read_receipts.rs

crates/matrix-sdk-base/src/store/traits.rs

jplatte · 2024-10-12T19:08:00Z

crates/matrix-sdk-common/src/executor.rs

+#[cfg(target_arch = "wasm32")]
+pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + 'a>>;
+#[cfg(not(target_arch = "wasm32"))]
+pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;


Suggested change

#[cfg(target_arch = "wasm32")]

pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + 'a>>;

#[cfg(not(target_arch = "wasm32"))]

pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + Send + 'a>>;

pub type BoxFuture<'a, T> = Pin<Box<dyn Future<Output = T> + SendOutsideWasm + 'a>>;

I see this definition already existed though, why did you move it here?

I needed to expose the .boxed method on the wasm version of the code (it was present on the real future, but not the Wasm version). It seemed like this was where the wasm future was being defined, so I wanted to move the BoxFuture method here so it can be exposed in the same place.

jplatte · 2024-10-12T19:08:28Z

crates/matrix-sdk-common/src/executor.rs

+#[cfg(not(target_arch = "wasm32"))]
+impl<'a, F, T> BoxFutureExt<'a, T> for F
+where
+    F: Future<Output = T> + 'a + Send,
+    T: 'a,
+{
+    fn box_future(self) -> BoxFuture<'a, T> {
+        self.boxed()
+    }
+}
+
+#[cfg(target_arch = "wasm32")]
+impl<'a, F, T> BoxFutureExt<'a, T> for F
+where
+    F: Future<Output = T> + 'a,
+    T: 'a,
+{
+    fn box_future(self) -> BoxFuture<'a, T> {
+        self.boxed_local()
+    }
+}


impl blocks can be merged the same way.

I was struggling to get it to compile with the extra + Send trait needed on the not-wasm32 version.

codecov · 2024-10-13T15:05:14Z

Codecov Report

Attention: Patch coverage is 42.85714% with 12 lines in your changes missing coverage. Please review.

Project coverage is 84.84%. Comparing base (8865e2f) to head (2592405).

Files with missing lines	Patch %	Lines
crates/matrix-sdk-common/src/executor.rs	56.25%	7 Missing ⚠️
crates/matrix-sdk-base/src/store/observable_map.rs	0.00%	4 Missing ⚠️
crates/matrix-sdk/src/sliding_sync/mod.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4122      +/-   ##
==========================================
- Coverage   84.89%   84.84%   -0.05%     
==========================================
  Files         272      271       -1     
  Lines       29167    29179      +12     
==========================================
- Hits        24761    24758       -3     
- Misses       4406     4421      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zzorba · 2024-10-13T19:31:49Z

With an additional change to relax the use of Send+Sync in the eyeball package, I have the rest of this crate building locally.

remaining.patch

poljar

Looks sensible, left a couple of nits.

crates/matrix-sdk-ui/src/timeline/event_item/local.rs

poljar · 2024-10-16T10:23:07Z

crates/matrix-sdk/src/event_handler/mod.rs

@@ -512,7 +509,7 @@ macro_rules! impl_event_handler {
        impl<Ev, Fun, Fut, $($ty),*> EventHandler<Ev, ($($ty,)*)> for Fun
        where
            Ev: SyncEvent,
-            Fun: FnOnce(Ev, $($ty),*) -> Fut + Clone + SendOutsideWasm + SyncOutsideWasm + 'static,
+            Fun: FnOnce(Ev, $($ty),*) -> Fut + Clone + Send + Sync + 'static,


Wait, why would you need Send + Sync unconditionally here?

Are you planning on continuing this PR or should we close it?

I restored the Send/Sync here -- until the eyeball change lands we can't complete a wasm build of this yet anyway, so there will need to be a second PR to handle stragglers at that point.

This commit adjusts types and traits to support targeting wasm32.

zzorba · 2024-11-05T14:37:11Z

Afraid I'm going to have to close this for now, will revisit in the new year when we have time to focus on this effort again.

maan2003 · 2024-11-06T17:10:13Z

I rebased my patches and bunch of new changes: https://github.com/matrix-org/matrix-rust-sdk/compare/main...maan2003:matrix-rust-sdk:push-rumyppqlqkkq?expand=0

But I don't have the energy to go through review :/

zzorba force-pushed the matrix-sdk-ui-support-wasm32 branch 2 times, most recently from 55f7b61 to 617c622 Compare October 12, 2024 18:48

jplatte reviewed Oct 12, 2024

View reviewed changes

zzorba force-pushed the matrix-sdk-ui-support-wasm32 branch from 617c622 to b93d3b7 Compare October 13, 2024 14:21

zzorba changed the title ~~Support compilation of wasm32-unknown-unknown for matrix-sdk-ui crate~~ Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate Oct 13, 2024

zzorba force-pushed the matrix-sdk-ui-support-wasm32 branch 5 times, most recently from 26943b1 to ac6b065 Compare October 13, 2024 14:49

zzorba marked this pull request as ready for review October 13, 2024 17:05

zzorba requested review from a team as code owners October 13, 2024 17:05

zzorba requested review from poljar and richvdh and removed request for a team October 13, 2024 17:05

poljar removed the request for review from richvdh October 16, 2024 09:54

poljar requested changes Oct 16, 2024

View reviewed changes

toger5 mentioned this pull request Oct 22, 2024

matrix-sdk-ui: compile on wasm #3333

Closed

Make the matrix-sdk-ui crate support wasm32-unknown-unknown

2592405

This commit adjusts types and traits to support targeting wasm32.

zzorba force-pushed the matrix-sdk-ui-support-wasm32 branch from b918f09 to 2592405 Compare November 5, 2024 14:26

zzorba closed this Nov 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate #4122

Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate #4122

zzorba commented Oct 12, 2024 •

edited

Loading

jplatte Oct 12, 2024

jplatte Oct 12, 2024

zzorba Oct 12, 2024

jplatte Oct 12, 2024

zzorba Oct 12, 2024

codecov bot commented Oct 13, 2024 •

edited

Loading

zzorba commented Oct 13, 2024

poljar left a comment

poljar Oct 16, 2024

poljar Nov 5, 2024

zzorba Nov 5, 2024

zzorba commented Nov 5, 2024

maan2003 commented Nov 6, 2024

Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate #4122

Start addressing some wasm32-unknown-unknown errors for matrix-sdk-ui crate #4122

Conversation

zzorba commented Oct 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Oct 13, 2024 • edited Loading

Codecov Report

zzorba commented Oct 13, 2024

poljar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zzorba commented Nov 5, 2024

maan2003 commented Nov 6, 2024

zzorba commented Oct 12, 2024 •

edited

Loading

codecov bot commented Oct 13, 2024 •

edited

Loading