vitejs / vite-plugin-react-swc

Speed up your Vite dev server with SWC
MIT License
781 stars 50 forks source link

HMR not working in a monorepo package that only exports shared functions #59

Closed KasimAhmic closed 1 year ago

KasimAhmic commented 1 year ago

Describe the bug

Currently working on migrating our monorepo to use Vite and am encountering an issue with HMR. When making changes in a package that only contains Redux logic, React hooks, and various utility functions, the entire app is reloaded. Making any changes in any other package results in HMR working properly.

The project structure follows this sort of structure:

Name Type Role Dependencies
@ahmic/vite-repro Root project None
@ahmic/commons Library Redux logic, hooks, utils None
@ahmic/components Library Re-exported MUI components and other UI components @ahmic/commons
@ahmic/app App User facing app @ahmic/commons and @ahmic/components

Additionally, I noticed this comment on another issue (https://github.com/vitejs/vite-plugin-react/issues/75#issuecomment-1368514653) that suggests HMR only works for files with components however, modifying hooks and pure functions in my @ahmic/app project results in HMR working just fine.

Reproduction

https://github.com/KasimAhmic/vite-repro

Steps to reproduce

  1. Run yarn run init
  2. Run yarn start
  3. Make a change in a hook and component in @ahmic/components
    • HMR works
  4. Make a change in a hook and component in @ahmic/app
    • HMR works
  5. Make a change in a hook in @ahmic/commons
    • HMR DOES NOT work, the page fully reloads instead

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
    Memory: 16.71 GB / 31.92 GB
  Binaries:
    Node: 18.12.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (110.0.1587.46)
    Internet Explorer: 11.0.19041.1566

Used Package Manager

yarn

Logs

No response

Validations

ArnaudBarre commented 1 year ago

The issue is that your main.tsx is too big and imports too many things. In App.tsx, create and export AppWithProviders. main.tsx should just mount it (and contains root side effect imports but there is none I can see in the repro)

KasimAhmic commented 1 year ago

Thanks for the fast reply @ArnaudBarre!

I moved the providers to App.tsx and my main.tsx looks like this now:

root.render(
  <React.StrictMode>
    <App />
  </React.StrictMode>
);

However, now when I make an update to @ahmic/commons, while the entire page doesn't reload anymore, all of the components from @ahmic/components are reloaded in addition to whatever was changed in commons. Thoughts?

ArnaudBarre commented 1 year ago

Yes that's the issue with barrels exports, any change in one file that is not a component will bubble up to the barrel exports file and then bubble up to every component importing this file. You have mutiple solutions:

If this is too much works, you can be still quite happy that HMR works across the monorepo :)

KasimAhmic commented 1 year ago

Ah I see. Option 1 wouldn't work for us as we need the package structure. Option 2 I don't really follow. Option 3 might work for us but would require a massive refactor of our imports across 7 different packages.

Do you know if there is any work planned to "fix" this? Or is this just a fundamental limitation of barrel exports and is just something we have to deal with?

ArnaudBarre commented 1 year ago

Option 2 means using import "../../commons/src/dir/useDummyHooks.ts" for the main app. But if components package need to have clean import from the common package, this would not be enough I think.

Theoretically this is something that could be solved by completely changing how files dependencies are tracked (being able to know that import { A } from "components" only depends on components/A/A.tsx). This would require more parsing (tracking and updating which files export what, be sure that intermediary files are side-effect free, ...).

KasimAhmic commented 1 year ago

I think that would be a great addition to Vite/ cause this import/export pattern is super convenient.

At any rate, due to the size of our components package, a full page reload is only marginally slower than hot reloading every component so we've decided to just go forward with that for now. Besides speed, there are also some weird connectivity issues that occur when we hot reload commons stuff.

Thanks a lot for your help!

P.S. I just realized I posted this issue here and not in the vite-plugin-react-swc repo. Is everything we discussed today valid for both the vite-plugin-react and vite-plugin-react-swc plugins or would it have been more appropriate to post there since I'm using the SWC variant? Or is this issue more-so for Vite itself?

ArnaudBarre commented 1 year ago

This is mostly related to how Vite handles HMR. The babel plugin has some differences that could make this pattern even worse.

I'm closing this as work as intended. Even if I understand that it would be great to support this pattern, this is almost impossible to integrate in the current state of Vite.

I'm planning to add caching to the the plugin at one point, this would help for this kind of updates that invalidates a lot of files without changing the transformation output.