search

vitejs / vite-plugin-react-swc

Speed up your Vite dev server with SWC
MIT License
830 stars 54 forks source link

refactor: simplify `$RefreshReg$` handling #204

Closed sapphi-red closed 4 months ago

sapphi-red commented 4 months ago

While reviewing #203 I found out that the code can be simplified.

Built on top of #203.

sapphi-red commented 4 months ago

The document says the following code needs to be injected.

  import RefreshRuntime from 'http://localhost:5173/@react-refresh'
  RefreshRuntime.injectIntoGlobalHook(window)
  window.$RefreshReg$ = () => {}
  window.$RefreshSig$ = () => (type) => type
  window.__vite_plugin_react_preamble_installed__ = true

So window.__vite_plugin_react_preamble_installed__ should be declared as long as they follow the document. The redundant window.$RefreshReg$/$RefreshSig$ won't be a problem. If we want to keep compat further so that it works with users that dropped window.__vite_plugin_react_preamble_installed__ = true, we can change window.__vite_plugin_react_preamble_installed__ = true to window.$RefreshReg$ = () => {} and if (window.__vite_plugin_react_preamble_installed__) { to if (window.$RefreshReg$) {.

ArnaudBarre commented 4 months ago

I just found that it breaks HMR on one of my utils that was manually registering the created itself with window.$RefreshReg$?.(Provider,${key}Provider); (Which could be improved on the transformation because the component is registered for hooks with $RefreshSig$);

I did a code search on GitHub but didn't find other usages, but given this is in the initial guide and that it's been used everywhere else I'm not sure it's good that when people come to Vite they have trouble because some lib they used was checking if $RefreshReg$ is in window as a proxy for dev for ex.

sapphi-red commented 4 months ago

Ah, that initial guide is a good point. I'll close this one.