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

Support fast refresh in workers #152

Closed
4 tasks done
itsdouges opened this issue Apr 24, 2023 · 9 comments
Closed
4 tasks done

Support fast refresh in workers #152

itsdouges opened this issue Apr 24, 2023 · 9 comments

Comments

@itsdouges
Copy link

Description

Currently this plugin assigns variables to window - which is unavailable inside workers. If instead we targeted globalThis as an alternative [...The globalThis property provides a standard way of accessing the global this value (and hence the global object itself) across environments.] that would be a way forward where the same code can function in both a DOM and worker environment.

Suggested solution

Replace all window usage with globalThis.

Alternative

No response

Additional context

No response

Validations

@ArnaudBarre
Copy link
Member

Do you React components inside your worker files?

@itsdouges
Copy link
Author

There's an experimental react three fiber api that runs scenes inside a worker, to simplify the model react is ran inside there as well.

That's why you see in another thread folks wanting to be able to disable fast refresh in 4.0. A better alternative is to keep it working imo.

@ArnaudBarre
Copy link
Member

Yeah but that's not trivial, you need to inject the fast refresh runtime exactly one per worker, and before any component run.

Can you provide a small setup with a component running inside a worker with a state so I can experiment with it?

Does react three fiber recommend using only one worker or multiple?

@itsdouges
Copy link
Author

itsdouges commented Apr 24, 2023

@drcmda would you be able to create one?

@drcmda
Copy link

drcmda commented Apr 24, 2023

@ArnaudBarre here's a vite example. https://github.com/pmndrs/react-three-offscreen/tree/main/examples/vite

due to #151 it only works with [email protected] atm.

@ArnaudBarre
Copy link
Member

Thanks I will look into it to understand why #151 happens.

For the feature request here, I will need to know if using multiple workers doing offscreen render is expected or not?

@drcmda
Copy link

drcmda commented Apr 24, 2023

thanks arnaud! i think not. it would be to allow a react root to run off the main thread and the result of its view is sent into a offscreen canvas. im guessing most react canvas renderers will only require a single worker, react-konva, react-skia, babylon, pixi, etc.

@siefkenj
Copy link
Contributor

I just encountered this issue. I am using React in a webworker to statically render markup via ReactDOM, and this plugin is currently crashing my webworker by using window. It would be neat to support refreshing in webworkers, but at a minimum, I would hope that the plugin could self-disable to avoid erroring in a webworker.

@ArnaudBarre
Copy link
Member

The assignment for window is now behind a check to not fail inside workers. The support for Fast refresh in web worker requires first Vite support for HMR inside web web workers: vitejs/vite#5396

I'm closing this for now, this update of the plugin will be straight forward if the feature lands into core (There is a stale but interesting draft PR for this)

@ArnaudBarre ArnaudBarre closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants