version 3 loses component local state

[jlongster]

I have an accordion that has local state that tracks which panes are open. When I update one of the panes, the open panes collapse because (I'm assuming) the accordion's state is being set back the default state.

There are many moving pieces so I'm not sure what to point out. I'm using webpack-dev-middleware, webpack-hot-middleware, and my config looks like this:

const config = Object.assign({}, projectConfig, {
  entry: [
    "webpack-hot-middleware/client?path=/__webpack_hmr&timeout=2000&quiet=true",
    "react-hot-loader/patch",
    path.join(__dirname, "../public/js/main.js")
  ]
});

config.plugins = config.plugins.concat([
  new webpack.HotModuleReplacementPlugin(),
  new webpack.NoErrorsPlugin()
]);

config.module.loaders = config.module.loaders.concat([
  { test: /\.js$/,
    exclude: /(node_modules|bower_components)/,
    loader: "react-hot-loader/webpack" }
]);

// ...

app.use(webpackDevMiddleware(compiler, {
  publicPath: config.output.publicPath,
  noInfo: true,
  stats: {
    colors: true
  }
}));

app.use(webpackHotMiddleware(compiler, {
  log: console.log,
  path: '/__webpack_hmr',
  heartbeat: 10 * 1000
}));

My component files are definitely being transformed, here is a snippet:

// ...
module.exports = connect(
      (state, props) => ({
        breakpoints: _getBreakpoints(state)
      }),
      dispatch => bindActionCreators(actions, dispatch)
    )(Breakpoints);


     ;(function () { /* react-hot-loader/webpack */ if (process.env.NODE_ENV !== 'production') { if (typeof __REACT_HOT_LOADER__ === 'undefined') { return; } if (typeof module.exports === 'function') { __REACT_HOT_LOADER__.register(module.exports, 'module.exports', "Breakpoints.js"); return; } for (var key in module.exports) { if (!Object.prototype.hasOwnProperty.call(module.exports, key)) { continue; } var namedExport = void 0; try { namedExport = module.exports[key]; } catch (err) { continue; } __REACT_HOT_LOADER__.register(namedExport, key, "Breakpoints.js"); } } })();
    /* WEBPACK VAR INJECTION */}.call(exports, __webpack_require__(17)))

Finally, how I render the root component:

function renderToolbox() {
  ReactDOM.render(
    React.createElement(
      AppContainer,
      null,
      React.createElement(
        Provider,
        { store },
        React.createElement(require("./components/TabList"))
      )
    ),
    document.querySelector("#mount")
  );
}

if(module.hot) {
  module.hot.accept('./components/TabList.js', () => {
    renderToolbox();
  });
}

Looking at AppContainer, I don't see where it would save local state. I'm requiring the new version of TabList which ends up requiring the new version of my component, and it appears to just be re-rendering my whole app with the new component, and I don't see where state is transferred across these instances.

[gaearon]

If you want to keep the local state of a connect()ed component, you need to either:

• Export the “inner” component so it’s visible in module.exports so react-hot-loader/webpack can pick it up;
• Or remove react-hot-loader/webpack and use react-hot-loader/babel in .babelrc instead, which can pick up any top-level variable as opposed to just the exports.

Does this make sense?

Looking at AppContainer, I don't see where it would save local state

It patches createElement so your component is resolved to a proxy that keeps the state. But for this to work, the component must be picked up either by /webpack (only works for exports), or by /babel (works better but requires Babel).

[jlongster]

This is the accordion component that I was tweaking, and it's not a connected component: https://gist.github.com/jlongster/5d97bac8c9d776d12a7e8b2c17a6b3af

The component which creates the accordion, however, is: https://gist.github.com/jlongster/204d05ca52874d099ea4ecf34ca23c15

I don't quite understand how the new version works though, so I might still be missing something.

[gaearon]

Aha. It seems like the same problem as #275 (comment).

We patch React.createElement() but React.DOM factories use ReactElement.createElement (internal React module) directly, and so bypass our patch.

Can you try replacing

require('react-hot-loader/patch')

with

require('react-hot-loader/patch')
require('react/lib/ReactElement').createElement = require('react').createElement

?

[gaearon]

Filed as #277, this is the most likely culprit.

[jlongster]

I was including the patch as a webpack entry, but I removed that and added both lines to the top of my main.js. I get a "too much recursion" error. Neither Firefox or Chrome is giving a stack to figure out where this is happening (Chrome is actually crashing with devtools open)

I'd poke around some more to help with this but I don't have a whole lot of time. Any idea why it would create an infinite loop?

[gaearon]

Ugh, you’re right. There’s some tricky dependency stuff going on there. This is the stack:

I can’t say for sure, but in any case monkeypatching internal modules is a bad idea.

I'd poke around some more to help with this but I don't have a whole lot of time. Any idea why it would create an infinite loop?

We need to patch createElement to always go through our version. It seems that patching React.createElement is not enough because React.createFactory and React.DOM.* reference the “inner”, unpatched createElement. The fix as I see it would be to also patch the exported React.createFactory and React.DOM.* factories to work through our createElement. Some more manual work but should be doable.

If you’d like to help, feel free to copy paste this test case, call it with React.createFactory() and React.DOM.* and change it to not use JSX. This will give you a failing test case. You can then leave it for me, or implement createFactory and .DOM.* patching in patch.dev.js.

[jlongster]

Alright, thanks for looking into it! I would love to help but I'm a little tight on time right now.

I just noticed something odd. I tried version 1.3.0 and I'm actually still seeing the behavior of the accordion collapsing. It looks like the state is being reset with the old version too, so while I may have highlighted a problem with the new version, I may be doing something wrong. I don't really know, the old version usually worked for me, so I'll have to look into what's up.

[gaearon]

Got it. Let’s keep it open for now until we know the root cause.

[calesce]

@jlongster any way can you see whether you still have this issue? createFactory is now patched (#287), and as far as I can tell React.DOM.* all go through createFactory, which should cover all of our bases (if that was, in fact, your problem).

[calesce]

Gonna close this out because I haven't heard back. Please re-open if you're still having the same issue!

[truongsinh]

https://github.com/Glavin001/react-hot-ts, as recommended by https://github.com/gaearon/react-hot-loader/tree/master/docs, is still losing internal state. Is it because of the fact that it uses purely TypeScript, and not TypeScript + Babbel?

@gaearon

• Export the “inner” component so it’s visible in module.exports so react-hot-loader/webpack can pick it up;
• Or remove react-hot-loader/webpack and use react-hot-loader/babel in .babelrc instead, which can pick up any top-level variable as opposed to just the exports.

[calesce]

@truongsinh: I don't know understand the semantics of private properties in TypeScript. It should work without Babel, but needs the Webpack loader. You can look here for a working example of that Counter example.

[truongsinh]

Thank you for the reference. private in TypeScript is only for type check, and can still be accessed at runtime using the same symbol. Unfortunately, I copied the whole

export default class Counter extends Component {
  constructor(props) {
    super(props);
    this.state = { counter: 0 };
  }

  componentDidMount() {
    this.interval = setInterval(this.tick.bind(this), 1000);
  }

  tick() {
    this.setState({
      counter: this.state.counter + 1
    });
  }

  componentWillUnmount() {
    clearInterval(this.interval);
  }

  render() {
    return (
      <h2>Counter: {this.state.counter}</h2>
   );
  }

But the Counter component still loses state. Here's the webpack.config.js (taken from https://github.com/Glavin001/react-hot-ts/blob/master/webpack.config.js)

const webpack = require("webpack");
const path = require("path");

module.exports = {
    entry: [
        "react-hot-loader/patch",
        "webpack-dev-server/client?http://localhost:3000",
        "webpack/hot/only-dev-server",
        "./src/index.tsx",
    ],
    output: {
        path: path.join(__dirname, 'dist'),
        filename: "bundle.js",
        publicPath: "/static/",
    },

    // Enable sourcemaps for debugging webpack's output.
    devtool: "source-map",

    resolve: {
        // Add '.ts' and '.tsx' as resolvable extensions.
        extensions: ["", ".webpack.js", ".web.js", ".ts", ".tsx", ".js"]
    },

    plugins: [
        new webpack.DefinePlugin({
            'process.env': {
                'NODE_ENV': JSON.stringify('production')
            }
        }),
        new webpack.HotModuleReplacementPlugin(),
    ],

    module: {
        loaders: [
            // All files with a '.ts' or '.tsx' extension will be handled by 'ts-loader'.
            {
                test: /\.tsx?$/,
                loaders: [
                    "react-hot-loader/webpack",
                    "awesome-typescript-loader"
                ],
                exclude: path.resolve(__dirname, 'node_modules'),
                include: path.resolve(__dirname, "src"),
            }
        ],

        preLoaders: [
            // All output '.js' files will have any sourcemaps re-processed by 'source-map-loader'.
            { test: /\.js$/, loader: "source-map-loader" }
        ]
    },

    // When importing a module whose path matches one of the following, just
    // assume a corresponding global variable exists and use that instead.
    // This is important because it allows us to avoid bundling all of our
    // dependencies, which allows browsers to cache those libraries between builds.
    externals: {
        "react": "React",
        "react-dom": "ReactDOM"
    },

};

[danzel]

For anyone who runs in to this, the solution for typescript for myself was to change the typescript loader to include "react-hot-loader/webpack"

See also:
Glavin001/react-hot-ts#2 (comment)
https://github.com/danzel/react-webpack2-typescript-hmr

[calesce]

Thanks @danzel, it'd be helpful to have a TypeScript section on the 3.0 docs.

[pocketjoso]

TLDR: don't use withRouter for the exported component used as as top level route component with react-router (and the Router)

Just wanted to report what was the problem in my case causing local state to be lost with RHL3:
The top level component we passed to our react-router Router component (same problem with react-router 3 and 4) was an App component that itself was exported via a withRouter(App) HOC (withRouter from react-router). I eventually discovered that this caused the App component to get re-mounted on every HMR cycle (save), causing the local state everywhere to be lost.

Solution for us:
Move the withRouter wrapping to sub components; don't use it with the top level component put inside the Router. Now HMR works and local state is kept.

Note for point of interest: we were, and still are, wrapping our App export in an connect - this does not cause any problem. Not sure exactly what withRouter does that causes this problem.

[pocketjoso]

Okay after fixing the problem above I realised a HOC we created ourselves breaks the HMR in the same way (always gets remounted so local state is always lost), I'm not quite getting why - if anyone has ideas:
https://gist.github.com/pocketjoso/deee25517824e8938a1956b212cfd3a0

[PeterKottas]

Did you @pocketjoso manage to fix this by any chance? I have quite a few HOC composed to get my final component and I am pretty sure I am hitting the same limitation. I could theoretically move the hoc functionality to some utils to (maybe) fix this, but damn, it should work with HOC as well. Shouldn't it?

[theKashey]

@PeterKottas With HOC - yes, with double HOC-no.
RHL will highlight the class you should try to export as a top-level variable.
Anyway - it is easy to solve your puzzle, than to explain that should you do.
Can you list the code with the problem?

[PeterKottas]

Hi @theKashey. My code looks something like this.

const roomsWrapped = withDashboardLayout(rolesLoadedHoc(propertyPlanLoadedHoc(roomsLoadedHoc(Rooms))));

The rooms component itself is a default export from a file and it's also connected to redux store. After reading bunch of issues here, it seems like what I have is a bit of a nightmare case.

Btw, withDashboardLayout is just an lambda component implemented like this.

const withLayout = (LayoutComp, ComponentComp, props) => <LayoutComp><ComponentComp {...props} /></LayoutComp>; const withDashboardLayout = (ComponentComp) => (props) => withLayout(DashboardLayout, ComponentComp, props);

I wonder if this would be causing problems as well. The rest of them are proper HOC.

[PeterKottas]

It might also be worth pointing out I am using typescript and awesome-typescript-loader to transpile. Setup is based on docs. Nothing special there.

[theKashey]

@PeterKottas - let me reveal the hidden truth

const roomsWrapped = 
withDashboardLayout( // produces a result
rolesLoadedHoc(   // produces a temporary variable
propertyPlanLoadedHoc( // produces a temporary variable
roomsLoadedHoc( // produces a temporary variable
Rooms // a variable.
))));

So - here we hot 3 temporal classes, RHL can not manage.
One said that https://www.npmjs.com/package/extract-hoc could help, or you can extract everything manually.

[PeterKottas]

Cool, that makes perfect sense. But it's still quite a problem for me :( Not exactly sure how to approach it as:
a) I am using swesome-typescript-loader so can't use that babel plugin
b) I have so many of these I'll tear half of my hair out before I'd manage to do it all manually.

was this always a no no with hmr? I could have sworn this was working at some point in time.
Maybe the principles of that plugin could be used to build a lib that could deconstruct these in a fashion that wouldn't force one to use babel. I'd have to think about it a bit.

For dev experience, I guess I could fake the initial state to "working state" to allow me to work with HMR, it's a bit of a pain though