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

seanaye/feat/js fetch #1554

Merged
merged 20 commits into from
Sep 25, 2023
Merged

seanaye/feat/js fetch #1554

merged 20 commits into from
Sep 25, 2023

Conversation

seanaye
Copy link
Contributor

@seanaye seanaye commented Aug 17, 2023

What is this

This PR adds good-enough support for targeting the wasm32-unknown-unknown target on the server.

This opens up the door for deploying to edge platforms such as Cloudflare workers, Deno Deploy, Netlify Edge, etc.

Really anything that supports the standard fetch API and wasm

Still TODO (another PR)

figure out the integration with cargo leptos this is currently pretty rough

Usage

  1. cd examples/hackernews_js_fetch
  2. deno task build
  3. deno task start

@seanaye
Copy link
Contributor Author

seanaye commented Aug 18, 2023

It looks like the CI is trying to compile the new example and its failing because the build step is different

@gbj
Copy link
Collaborator

gbj commented Aug 18, 2023

The changes made to the Axum integration here (removing get_leptos_pool and spawn_pinned) cause a panic at runtime when running the rendering functions or executing server functions.

thread 'tokio-runtime-worker' panicked at '`spawn_local` called from outside of a `task::LocalSet`', /Users/gjohnston/Documents/Projects/leptos-main/leptos/leptos_reactive/src/spawn.rs:85:13

@seanaye
Copy link
Contributor Author

seanaye commented Aug 18, 2023

oh I thought that was being tested by CI and passing, my mistake. I'll add a cfg if block because creating the pool causes wasm to panic

@seanaye
Copy link
Contributor Author

seanaye commented Aug 18, 2023

I am testing the server fns handling, it looks like the client and the server are generating different ids for the same function

fixed

@seanaye
Copy link
Contributor Author

seanaye commented Aug 22, 2023

@gbj I think I might need some help getting the CI to play nice with the new example. Something with the matrix of different features is causing cargo clippy to fail

@gbj
Copy link
Collaborator

gbj commented Aug 23, 2023

  1. This is amazing — can confirm the example works.
  2. I'm sure there's some additional cleanup that needs to be done (warnings etc.) to make the CI happy, can deal with that later.
  3. The new reactive implementation uses tokio task-local variables to store the current reactive runtime per thread, to make sure there's no bleeding between requests. I'll probably need to do some testing to make sure that this works with server-side WASM, as using wasm_bindgen_futures::spawn_local opts out of the task-local runtime guarantees we're now able to provide with tokio task-local variables if there are multiple concurrent requests to the server.

@gbj
Copy link
Collaborator

gbj commented Sep 22, 2023

Okay, sorry it's taken me a long time to get to this. It turns out that my point 3 above was wrong: I have run my standard test for this and it seems to work fine, without leakage across runtimes, and without any modifications. I suspect it has to do with the JS/WASM environment being single-threaded in any case: is Deno spinning up additional threads and running different JS runtimes in them for different requests?

The test, for future reference, and in case I screwed it up
  1. Create a simple page with an element that synchronously uses {format!("{:?}", current_runtime())}
  2. Add a server function that delays for one second, then returns the current runtime ID (formatted to a string), and render this in a Suspense
  3. Spam the page with something like ab -n 512 -c 16 http://127.0.0.1:8000/
  4. If the two RuntimeId values match, then we're not leaking between runtimes: each request is successfully running with a single RuntimeId

I'm honestly not sure what's going on with the CI. Digging into it a bit. It has to do with the combo wasm+SSR combo and feature flags, I'm sure.

@seanaye
Copy link
Contributor Author

seanaye commented Sep 25, 2023

No, Deno will not spin up worker threads by default but in theory it would be possible. The rust futures do get run via the JS event loop though so it is 'non-blocking' in a sense, for IO-bound operations at least. I think that as long as we aren't leaking the runtime then leptos has fulfilled its end of the bargain, it would be up to the caller to do worker threads if desired. This would be separate WASM instances so if 1 runtime doesn't leak then N runtimes also wont leak.

Regardless, I think the main usecase for this PR is edge runtimes like Cloudflare Workers where threading is already abstracted away.

@gbj gbj merged commit 50678da into leptos-rs:main Sep 25, 2023
55 checks passed
Comment on lines +73 to +75
ClearServerCount::register_explicit().unwrap();
AdjustServerCount::register_explicit().unwrap();
GetServerCount::register_explicit().unwrap();
Copy link

Choose a reason for hiding this comment

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

@seanaye @gbj
hey there again. I have a question about why register_explicit is needed. Is it not supported for wasm32 servers yet?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@9oelM Correct, automatic registration isn't and can't be supported on WASM targets, and unless I'm mistaken that's documented in the server function docs.

Copy link

Choose a reason for hiding this comment

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

yeap got that thanks for the answer @gbj !!

@g2p
Copy link
Contributor

g2p commented Sep 30, 2023

@seanaye, the axum-js-fetch repo seems to be private, could you make it available?

@seanaye
Copy link
Contributor Author

seanaye commented Sep 30, 2023

@seanaye, the axum-js-fetch repo seems to be private, could you make it available?

Yes of course, my mistake, I will do so later today

@seanaye
Copy link
Contributor Author

seanaye commented Sep 30, 2023

@g2p https://github.com/seanaye/axum-js-fetch PRs are welcome

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.

4 participants