search

vitejs / vite-plugin-react

The all-in-one Vite plugin for React projects.
MIT License
546 stars 102 forks source link

fix: define stubs for react-refresh in Web Worker #311

Closed eyr1n closed 1 month ago

eyr1n commented 2 months ago

Description

This PR fixes the issue below and improves #181. Also the issue is mentioned in https://github.com/vitejs/vite/issues/5396#issuecomment-2050127910

Define some stubs in Web Worker which is required by react-refresh to avoid the error: Uncaught ReferenceError: $RefreshReg$ is not defined.

Issue

First, create Worker.tsx with the following content

function Worker() {
  return <></>;
}

Then, load Worker.tsx from App.tsx

const worker = new Worker(new URL("./Worker.tsx", import.meta.url), {
  type: "module",
});

In that case, we got the error below

Uncaught ReferenceError: $RefreshReg$ is not defined

Additional context

To avoid this error, @react-three/offscreen package, which renders React Three Fiber to OffscreenCanvas, shows the workaround to downgrade vite-plugin-react and disable all HMR. This provides bad development experiences, so I hope that a new version will be released soon.

Reference: https://github.com/pmndrs/react-three-offscreen?tab=readme-ov-file#vite

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

eyr1n commented 1 month ago

Can anyone review this?

ArnaudBarre commented 1 month ago

Does this feature is used to render part of a React tree of the whole tree? Because as said here the v3 hack is actually just using raw Vite without plugin. Do you expect any HMR to work after this PR?

eyr1n commented 1 month ago

@ArnaudBarre We are hoping that a part of the React tree will render correctly, not the whole React tree.

I'm using @react-three/offscreen on a small part of my application configured with Vite and don't expect HMR to work correctly in Web Worker because I don't make many changes to this part(offscreen) of the application.

However, I believe that disabling HMR for the whole application just because of packages like @react-three/offscreen would lose value. Therefore, I believe that providing such a stub while HMR is not yet implemented in Web Worker will help many users.

I'm sorry, it was just my project configuration that was wrong, I had missed a check for a file to be used in the worker, and once I properly excluded it, the code worked correctly. You may close this PR.

ArnaudBarre commented 1 month ago

For anyone coming here, please provide a example of how adding this kind of check could lead to a better DX than just having bare Vite without plugins (which was what people experienced with v3 + disableFastRefresh)

cpojer commented 1 month ago

@ArnaudBarre The problem is that when using the worker plugin, Vite may choose to import modules with JSX in a worker (whether they are used or not). The worker then fails because those variables are not initialized in the worker. The only way to solve this is by adding a side-effectful module that you import before any other modules in a worker.

See initializeWorker.tsx and the example usage in a worker.

I don't think there are any downsides for the Vite React plugin to define the global variables in a worker if import.meta.hot is provided.

ArnaudBarre commented 1 month ago

Are you running React component inside a worker or does JSX end up in the worker bundle because of imports being too wide?

cpojer commented 1 month ago

I assume most people run into the latter. In my case I didn't include any React components, but Vite chose to bundle them in development anyway (??).

ArnaudBarre commented 1 month ago

Yeah some people run in the latter, often because of barrel files, which as always been something that doesn't works well with unbundled dev. Changing this will improve development speed and reduce the number of times you get HMR for code that is actually unused at runtime.

I'm a bit afraid that adding no-op for HMR now will make it a breaking change if at one point someone find a way to have HMR working for workers.