vite-plugin-react icon indicating copy to clipboard operation
vite-plugin-react copied to clipboard

HMR not working when lazy loading Class component wrapped in Hoc

Open froleo opened this issue 2 years ago • 12 comments

Describe the bug

Trying to migrate an older create-react-app to Vite i ran in to the following issue:

When a Class component is lazy loaded using React.lazy and the component is wrapped in a Hoc then HMR stops working. The browser has to be refreshed manually to show changes.

When React.lazy is removed HMR works. Also when using @loadable/component instead of React.lazy HMR works.

https://user-images.githubusercontent.com/12452257/229728112-b4fcd373-fe74-4c5c-8640-7aeddd820100.mp4

See minimal reproduction in codesandbox below:

Reproduction

https://codesandbox.io/p/sandbox/elated-dawn-tvd36y?file=%2Fsrc%2FApp.tsx

Steps to reproduce

  • Open codesandbox
  • Edit the file ClassComponentWithHoc.tsx
  • Save the changes

Result: The browser will not update the output unless you manually click the refresh button of the browser window.

System Info

System:
    OS: Linux 5.15 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    CPU: (2) x64 AMD EPYC
    Memory: 409.07 MB / 1.63 GB
    Container: Yes
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.15.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.5.0 - /usr/local/bin/npm
  npmPackages:
    @vitejs/plugin-react: ^3.1.0 => 3.1.0 
    vite: ^4.1.1 => 4.2.1

Used Package Manager

npm

Logs

No response

Validations

froleo avatar Mar 30 '23 15:03 froleo

Hello, just to let you know that I can repro and I spent already an hour looking into the internals of React.lazy and why this different from loadable but didn't find anything conclusive for now. Because CRA bundles source code more than Vite does, HMR behaves a bit differently. This should be fixable but I need to find more time to investigate

ArnaudBarre avatar Apr 14 '23 10:04 ArnaudBarre

I have a similar case, that is using HOC without lazy loading. For that I tried to integrate scrivito into vite.

How to reproduce

  • Clone https://github.com/apepper/vite_scrivito_hot_module_replacement
  • npm i
  • npm run dev (this starts vites dev server)
  • Visit http://127.0.0.1:5173/ in your browser.
  • Modify src/Objs/Homepage/HomepageComponent.tsx, e.g. by adding <span>foo</span> to the markup.
  • Modify src/Widgets/HeadlineWidget/HeadlineWidgetComponent.tsx, e.g. add a <span>bar</span> to the markup.

Both modifications should be automatically reflected in the browser. But currently only the HomepageComponent.tsx is updated, where as HeadlineWidgetComponent.tsx is not.

apepper avatar Apr 17 '23 11:04 apepper

I was able to fix the issue in scrivito. The next version will support vite even better.

apepper avatar May 03 '23 13:05 apepper

Hello, just to let you know that I can repro and I spent already an hour looking into the internals of React.lazy and why this different from loadable but didn't find anything conclusive for now. Because CRA bundles source code more than Vite does, HMR behaves a bit differently. This should be fixable but I need to find more time to investigate

@ArnaudBarre Have you been to find anything more on this? I'm eagerly awaiting to move over to vite, but this issue has been holding me off for now.

froleo avatar Jun 09 '23 12:06 froleo

I did not have time for digging into this, but I will give it another shot this weekend

ArnaudBarre avatar Jun 19 '23 23:06 ArnaudBarre

There is still something that I don't fully understand, maybe I will find a solution later but for now you have two main solutions:

  • Add names to your export so that fast refresh is applied on the HOC. You can use https://github.com/ArnaudBarre/eslint-plugin-react-refresh to find anonymous exports in your codebase
  • Make the default export a class component. You can create a useless HOC that take any component and just re-render it inside like this:
const fixHMR = (Cmp) =>
  class FixHMR extends Component {
    render() {
      return <Cmp />;
    }
  };
  
export default fixHMR(withStyles(styles)(ClassComponent));

This first solution is highly encouraged as this will also improved the interaction with other tools like the React devtools.

ArnaudBarre avatar Jun 25 '23 19:06 ArnaudBarre

Thank you for looking into this!

I could not get your first point to work. I tried something like this with a named export: export const Test = withStyles(styles)(ClassComponentWithHoc);

The problem is that React.lazy only accepts default import. So I tried this to import the named export: const ClassComponentWithHoc = lazy(() => import("./ClassComponentWithHoc").then((module) => ({ default: module.Test })) );

But this has the same result, update only works on manual refresh

Your fixHMR HOC works, so I have this as a workaround for now

froleo avatar Jun 30 '23 14:06 froleo

IIRC you need to do:

const ClassComponentWithHoc = withStyles(styles)(ClassComponent);
export default ClassComponentWithHoc;

ArnaudBarre avatar Jul 02 '23 21:07 ArnaudBarre

IIRC you need to do:

const ClassComponentWithHoc = withStyles(styles)(ClassComponent);
export default ClassComponentWithHoc;

This was one of the first approaches i tried, but it has the bug as well. Reproduced here: https://codesandbox.io/p/sandbox/zealous-snyder-hrp4jp?file=%2Fsrc%2FClassComponentWithHoc.tsx%3A16%2C73

froleo avatar Jul 03 '23 12:07 froleo

When we transformed our app from Create React App to Vite we also faced this issue. However, we noticed that if we add an empty functional component to a module containing a class component, it starts reloading changes after save.

This example doesn't work:

import { withStyles } from "@mui/styles";
import { Component } from "react";
const styles = (theme) => ({
  text: {
    color: "red",
    padding: "10px",
    border: "1px solid black",
  },
});

// HMR not working
class ClassComponentWithHoc extends Component {
  render() {
    const { classes } = this.props;

    return <p className={classes.text}>{`I am a class component with Hoc.`}</p>;
  }
}

export default withStyles(styles)(ClassComponentWithHoc);

But this does:

import { withStyles } from "@mui/styles";
import { Component } from "react";
const styles = (theme) => ({
  text: {
    color: "red",
    padding: "10px",
    border: "1px solid black",
  },
});

// Here is an empty function. This single line is enough, there is no need to change anything else.
const EmptyComponent = () => null;

// HMR now works!
class ClassComponentWithHoc extends Component {
  render() {
    const { classes } = this.props;

    return <p className={classes.text}>{`I am a class component with Hoc.`}</p>;
  }
}

export default withStyles(styles)(ClassComponentWithHoc);

We decided to create a Vite plugin to help overcome this problem. You can find it here. It adds an empty component for each module containing React class components. :slightly_smiling_face: Of course it works only in the development mode so you don't have to worry about your production code.

Here is a modified @froleo's example with the plugin added. The only difference is applying fix-react-refresh-plugin to vite.config.js.

marlo22 avatar Feb 23 '24 14:02 marlo22

@marlo22 I tested out your plugin now, works perfectly! 👏 Great to have a solution were I don't need to manually add workaround code all over the codebase. After nearly a year of holdout I can finally finish the Vite migration. Really appreciate it!

froleo avatar Mar 01 '24 15:03 froleo

@froleo glad to help you. :slightly_smiling_face:

Btw. there is a small bug in our plugin - if you run vite build, you'll get many warnings about sourcemaps. We'll release a fixed version next Monday, but for now you can bypass it by adding to your vite.config.js one line:

export default defineConfig({
  plugins: [
    {
      ...fixReactRefresh(),
      apply: 'serve', // Add this line.
      enforce: 'pre',
    },
    react()
  ]
});

Edit: we've released the update - v1.0.2

marlo22 avatar Mar 01 '24 18:03 marlo22