gaearon / react-hot-loader

Tweak React components in real time. (Deprecated: use Fast Refresh instead.)
http://gaearon.github.io/react-hot-loader/
MIT License
12.26k stars 801 forks source link

Entire DOM is re-rendered when just a single component is changed #1016

Open RonanQuigley opened 6 years ago

RonanQuigley commented 6 years ago

If you are reporting a bug or having an issue setting up React Hot Loader, please fill in below. For feature requests, feel free to remove this template entirely.

Description

React hot loader seems to re-render the entire tree of components when just one component has been changed. It also does this when you make use of multiple apps, each with their own separate roots.

Expected behavior

From my understanding, only the components that were updated should be changed, not unrelated apps or sibling components.

Actual behavior

Consider the GIFs below (this example just uses one container with twol apps inside of it; see the code at the bottom of this post):

example

Here i'm using two apps with a shared parent, adjusting each components background colour. I'm using why did you update to check this in the logger, and it shows that both apps are updated when only one component has been altered. This is what the output is in the console:

capture

In case this is a false positive: this is also evident in react dev tools with highlight updates:

example-2

This seems to be the most related issue I could find from the posted issues:

https://github.com/gaearon/react-hot-loader/issues/508

The reason I'm bringing this up is performance. The example here is trivial to make it obvious what's going on. In large scale projects, with 100s of components, I've found that this causes hot reloading to tank chrome's performance, with it taking several seconds to complete a simple style change. This is of course a big downer on productivity. Is this just how react-hot-loader is supposed to work? If so, why? I've checked out your documentation, but nothing comes up about it. I'm currently mitigating for it by just rendering the current app I'm working on to the browser.

Environment

React Hot Loader version: 4.3.1

Run these commands in the project folder and fill in their results:

  1. node -v:10.1.0

  2. npm -v: 6.0.1

  3. Operating system: Windows 10

  4. Browser and version: Chrome Canary 69.0.3453.0

Reproducible Demo

This is the example code being used. It was ran with webpack-dev-server.

// app-one.js

import React from "react";
import { hot } from "react-hot-loader";

function AppOne() {
  return <h1 style={{ backgroundColor: "red" }}>I am app one</h1>;
}

export default hot(module)(AppOne);

// app-two.js

import React from "react";
import { hot } from "react-hot-loader";
function AppTwo() {
  return <h1 style={{ backgroundColor: "white" }}>I am app two</h1>;
}

export default hot(module)(AppTwo);

// Root.js

import React from "react";
import AppOne from "./AppOne";
import AppTwo from "./AppTwo";
import { hot } from "react-hot-loader";

const Root = () => (
  <React.Fragment>
    <AppOne />
    <AppTwo />
  </React.Fragment>
);

export default Root;

// index.js

import React from "react";
import { render } from "react-dom";

import Root from "./Root";

import { whyDidYouUpdate } from "why-did-you-update";

whyDidYouUpdate(React);

render(<Root />, document.getElementById("root"));

And this is the babelrc :

{
  "presets": [
    [
      "latest", {
        "es2015": {
          "modules": false
        }
      }
    ],
    ["react"]
  ],
  "plugins": [
    "react-hot-loader/babel"
  ]
}

I've also replicated this with classes, using PureComponent vs Component, using componentShouldUpdate -> false, and having each app in its own container. It's the same in all cases. In my current setup I'm doing server-side rendering so I've also replicated it with ReactDOM.hydrate. I just used babel 6 for this example, but it also happens with babel 7.

theKashey commented 6 years ago

So that is not a bug. But I have no idea why both application got rendered. And there is no thing like "I've updated only one component" - you are updating everything "below" hot. I honestly thought that "hot-reloading" of application should take maximum 2x time from normal render, and thus be blinking fast. And, during the hot update, RHL will forcely render components 2 or more times, just to detect possible changes.

So - everything here, except secondary Application re-render is not a bug, and even not bound to RHL, cos the "size" of update comes from webpack.

RonanQuigley commented 6 years ago

Thanks for the clarification.

Perhaps I'm misunderstanding how react & hot reloading works here, but why should every single component need to be updated below a child component if it's a basic style change like changing a colour?

Yeah, if it's not a bug, it's still an insane performance hit. This is what happens when I change just the width of a div in one component in my current project:

react

Your "force render components 2 or more times" bit might explain then when this is happening in my current application. Is there anyway to get around this performance problem? At the least, as you said, both applications should not be re-rendered. This also extends to applications rendered in separate react containers.

So - everything here, except secondary Application re-render is not a bug, and even not bound to RHL, cos the "size" of update comes from webpack.

I'm not sure what you mean by this, could you be more specific?

EDIT

Just to add, I've noticed that this issue is also compounded with higher order components. Each additional one creates an additional call to render.

theKashey commented 6 years ago

Yeah, if it's not a bug, it's still an insane performance hit. This is what happens when I change just the width of a div in one component in my current project:

But that will update ALL components below hot. Or just all, if you have a single hot.

Is there anyway to get around this performance problem?

About this behavior - no. About performance problem - might be.

Each additional one creates an additional call to render.

That's the deal! Please activate debug mode ( setConfig({ logLevel: 'debug' }) ) RHL render application once, and then React render it. Total - 2 renders. If some part of application were not renderer by RHL, but then React renders them... RHL renders branch again. Like a oh, crap, I did I miss it.... Our way to handle portals and some edge cases. In terms of RHL this called subrender.

I wonder - might be the "main" render was not successful, due to an exception, and thus subrenders took a place?

RonanQuigley commented 6 years ago

Tried adding the logger in my app, but there's no real difference in the output. That could just mean there is nothing to log via debug though. The only time I can ever get debug logging info is if I were to remove some of the apps from the container by just commenting them out. I'll get something like this:

react-hot-loader.development.js:168 React-hot-loader: a withState(WithStyles(Charts)) was found where a Component was expected.
          class withState extends _react.PureComponent {constructor(...args) {var _temp;return _temp = super(...args), _defineProperty(this, "state",

      {
        value: 0 }), _defineProperty(this, "onChange",

      (event, value) => {
        this.setState({ value });
      }), _temp;}

    render() {
      const props = _extends({}, this.props);
      const { value } = this.state;

      return (
        _react.default.createElement(WrappedComponent, _extends({
          value: value,
          onChange: this.onChange },
        props)));

    } // @ts-ignore
    __reactstandin__regenerateByEval(key, code) {// @ts-ignore
      this[key] = eval(code);}}

But I don't think that has anything to do with the problem.

But that will update ALL components below hot. Or just all, if you have a single hot.

Yes but that doesn't explain why it happens 😄

My understanding is that react updates only what needs to be updated in the tree i.e. this is what is happening in my large app I'm working on:

example-react

I'm just doing a simple style change and only the elements which are effected by that change update in the tree at least.....

So would this issue happen even without react-hot-loader?

About this behavior - no. About performance problem - might be.

Hmm okay then what has your strategy been when developing apps that use arrays of child components? Has this been an issue you've come up against before?

theKashey commented 6 years ago

react-hot-loader.development.js:168 React-hot-loader: a withState(WithStyles(Charts)) was found where a Component was expected.

Is a fatal bug, not a warning :). RHL will not dive into the HOC, but will "force update" all components before. Probably there is some problem with displayName as long Component is "name-less" component.

Could you stop at the line, where that error emitted, and send me details.

https://github.com/gaearon/react-hot-loader/blob/master/src/reconciler/hotReplacementRender.js#L407

I need childType and stackChild.type. They are not swappable as long got different names, but are they actually the same, or RHL "render" process go wrong?

RonanQuigley commented 6 years ago

I think I'm misunderstanding what you're asking here, but I mean literally all I'm doing is commenting out a set of components, so I don't see why that could be related to this problem? This is the code I was commenting parts out:

<React.Fragment>
    <CssBaseline />
    // this was commented out and the warning shows up
    {/* <div className={classes.topChartsContainer}>
        <div id="artists" className={artists}>
            <Charts {...childProps.artists} />
        </div>
        <div id="tracks" className={tracks}>
            <Charts {...childProps.tracks} />
        </div>
    </div>*/}
    <div id="audio-features-container">
        <Pie {...childProps.key} />
        <Pie {...childProps.mode} />
        <Polar {...childProps.average} />
    </div>
</React.Fragment>

I do have a HOC for managing state like so:


export default function(WrappedComponent) {
    class withState extends PureComponent {
        static propTypes = {
            classes: PropTypes.object
        };

        state = {
            value: 0
        };

        onChange = (event, value) => {
            this.setState({ value });
        };

        render() {
            const { ...props } = this.props;
            const { value } = this.state;

            return (
                <WrappedComponent
                    value={value}
                    onChange={this.onChange}
                    {...props}
                />
            );
        }
    }
    // withState.displayName = `withState(${getDisplayName(WrappedComponent)})`;
    return withState;
}

I commented out the displayName code as I thought that was what you were referring to as causing the bug in your post. But it makes no difference.

I need childType and stackChild.type. They are not swappable as long got different names, but are they actually the same, or RHL "render" process go wrong?

I don't know what you mean here, sorry.

theKashey commented 6 years ago

Currently, RHL could work "strangely" if you add or remove components on the fly, as long "new" and "old" tree will be different. I should investigate this situation.

RonanQuigley commented 6 years ago

Sure, but what about the other issue? Maybe connected or separate?

theKashey commented 6 years ago

Don't know yet.

It should be:

  1. Ok
  2. Way faster than webpack part of the work.

I am more concerned about secondary application render, and your HOC behaviour.

RonanQuigley commented 6 years ago

Hmm well in this case it's not. My webpack rebuilds currently take around half a second. If I use all apps the hot reloading react on the client is about double that. Commenting out child components brings that down

RyanCCollins commented 5 years ago

Forgive me for hijacking this issue, but I feel I have a related issue.

I'm working in a rather large-scale react application for my job. We are using react-hot-loader and it has been not working for a long time (fully reloads the browser on each update). For each of our module folders (Screens, Components, etc.) we export the modules in that directory in a root index.js file and then re-import them across our application's features.

The only thing that I've found that fixes our hot-reloading is adding the hot HOC to any module I want to hot reload. For example, I've added them to our Screens in one of our features and those screens will now hot reload. We have hundreds (maybe even close to thousands) of modules/components, so it would not at all be feasible to have to go add the HOC to every component. Furthermore, it seems that our non-component files will not hot reload either (selectors, utils, reducers, etc.) Is this expected behavior in RHL? Is there anything else we can do that will make RHL "Just Work" for our application?

theKashey commented 5 years ago

Something inside your application is breaking RHL. Could you try enable debug and look for errors?