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

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

Closed
wants to merge 1 commit into from

Conversation

eyr1n
Copy link

@eyr1n eyr1n commented Apr 29, 2024

Description

This PR fixes the issue below and improves #181.
Also the issue is mentioned in vitejs/vite#5396 (comment)

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?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

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

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@eyr1n
Copy link
Author

eyr1n commented May 8, 2024

Can anyone review this?

@ArnaudBarre
Copy link
Member

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
Copy link
Author

eyr1n commented May 8, 2024

@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
Copy link
Member

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)

@ArnaudBarre ArnaudBarre closed this May 8, 2024
@eyr1n eyr1n deleted the fix/webworker branch May 8, 2024 20:47
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.

None yet

2 participants