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

Version 4 should still support disabling fastRefresh #151

Closed
7 tasks done
kherock opened this issue Apr 21, 2023 · 9 comments
Closed
7 tasks done

Version 4 should still support disabling fastRefresh #151

kherock opened this issue Apr 21, 2023 · 9 comments

Comments

@kherock
Copy link

kherock commented Apr 21, 2023

Describe the bug

I'm using Vitest to test my React code, and it does this by hosting a Vite dev server without a HTML entrypoint. With fast refresh automatically enabled, I get this error

Error: @vitejs/plugin-react can't detect preamble. Something is wrong. See https://github.com/vitejs/vite-plugin-react/pull/11#discussion_r430879201

Maybe there's a way to get fast refresh to work on Vitest with my test rendering setup, but I'd rather just disable it since I don't need it.

Reproduction

This should be reproducible on a basic vitest project with @testing-library/react. I'll include one in the coming days if this report seems wrong.

Steps to reproduce

No response

System Info

System:
    OS: macOS 12.6.3
    CPU: (10) arm64 Apple M1 Pro
    Memory: 115.97 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.19.1 - /private/var/folders/9f/kl_c86q5651fqmhmzf63jb0x5t_d9j/T/xfs-79f49cc3/node
    Yarn: 3.5.0 - /private/var/folders/9f/kl_c86q5651fqmhmzf63jb0x5t_d9j/T/xfs-79f49cc3/yarn
    npm: 8.19.3 - ~/.local/share/homebrew/bin/npm
  Browsers:
    Firefox: 102.2.0
    Safari: 16.3
  npmPackages:
    vite: ^4.3.1 => 4.3.1

Used Package Manager

npm

Logs

No response

Validations

@ArnaudBarre
Copy link
Member

It's strange for me that import.meta.hot is defined in your setup. The condition that throws is behind that check to not trigger in test context: 7c38c7e

In the SWC plugin I don't include this block at all when not in a HMR context code

My idea is that this testing mode should be detected by the plugin, so everyone using it don't have to configure it.

If you could provide a small repro it would help to craft the right condition to handle your use case

@github-actions
Copy link

Hello @kherock. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with need reproduction will be closed if they have no activity within 3 days.

@drcmda
Copy link

drcmda commented Apr 22, 2023

i have the same problem trying to run react (and jsx) in a web worker pmndrs/react-three-offscreen#2

it tries to inject something into an environment that doesn't have window and document defined. getting the same preamble error. previously the solution was to disable fast refresh: https://stackoverflow.com/questions/73815639/how-to-use-jsx-in-a-web-worker-with-vite

@drcmda
Copy link

drcmda commented Apr 22, 2023

the workaround currently is to use 3.x :(

https://github.com/pmndrs/react-three-offscreen#vite

...

  1. yarn add @vitejs/plugin-react@3.1.0
  2. disable fast refresh (see: stackoverflow) (the option was removed in 4.x)
export default defineConfig({
  plugins: [react({ fastRefresh: false })],
  worker: { plugins: [react()] },
})

i also don't understand why it has to be disabled in plugins and not just in worker.

@ArnaudBarre
Copy link
Member

ArnaudBarre commented Apr 25, 2023

So this is quite tricky.
The issue is that jsx component -> react-refresh -> hot context -> vite client -> expect to run in main thread. And always has been in Vite. I'm surprised people just disabled fast refresh and called it a day without checking if the issue was tracked somewhere.

And what's funny is that using react({ fastRefresh: false }) in v3 actually makes the plugin a no-op (that's the reason I removed it), so your current setup would just work by removing the config file & the dependency to the plugin 😅

There are two solutions to fix this:

  • Support the notion of HMR in workers in Vite core (and then makes some updates in the React plugins). I'm not sure the complexity of having multiple clients per browser tab would be easy to handle.
  • Have the plugin disable fastRefresh when the context is a worker. The issue is that the same module can be in a "window" context and a worker context at the same time (what happens with the fallback in the repro from @drcmda). And this is why you need to disable fast refresh also outside or workers to unsure the worker doesn't use the transformation output made for the fallback. Vite already track ssr transformations separated from "client" transformation. Maybe we could do the same for workers.

I will discuss both options wit the team.

@nstepien
Copy link

We have the same issue with @vitest/browser, I found a workaround:

  • in tests, disable the @vitejs/plugin-react plugin
  • enable the @rollup/plugin-babel plugin instead

Seems to work fine. 🤷‍♂️
For reference: adazzle/react-data-grid@6f2dc5f

@ArnaudBarre
Copy link
Member

Yeah still not sure how to best support test mode by default. More complex given that the testing environment can now also be a browser. I'm surprised you need a rollup plugin though. The default esbuild plugin from Vite core should be enough.

@nstepien
Copy link

@ArnaudBarre Ah you're correct, I just had to tweak the tsconfig's jsx setting.

@ArnaudBarre
Copy link
Member

Two of the current needs for this (web workers and testing) have been addressed in the latest version. For a real support of HMR inside web worker see vitejs/vite#5396

@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.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants