RFC: remove React Transform from examples

[gaearon]

Inspired by @thejameskyle’s rant on Twitter (no offense taken 😄 ), I propose to remove babel-preset-react-hmre from examples for the following reasons:

1. Both plugin transform and proxy wrapping are rather invasive and have edge cases
2. It’s experimental tech and beginners shouldn’t have to deal with its warts
3. It doesn’t currently reload functional components or container functions like mapStateToProps
4. In Redux we rarely care about preserving component local state when hot reloading! Preserving store state is usually enough.

This is a proof of concept of “vanilla” hot reloading (for now, only for a single example).

It has the following traits.

Pros

1. It is absolutely non-invasive and doesn’t change semantics of your code
2. It hot reloads any components, whether classes or functions
3. It preserves the store state on hot reload
4. It still handles errors on mount and hot reloading “fixes” them
5. It works for functions like mapStateToProps

Cons

1. It does not preserve the local state of components
2. It does not preserve the DOM so the components are remounted

What do you think? Should we proceed with this and do this for every example?

[17cupsofcoffee]

I think this is a good idea - having the transform plugin in the examples sorta gives the impression that it's something that you need to have in order to get hot reloading working with Redux. In fact, I was kinda under that impression myself until reading this!

[gaearon]

Yeah a lot of people tend to get that impression.

[ghost]

Given all of this talk about "JavaScript fatigue", the less dependencies you need, the better.

That said, I definitely want to see react-transform advertised heavily as a DX improvement/plugin for redux - once you get comfortable using vanilla redux.

[jamiebuilds]

Why not put the try-catch here and call render() below so even if you reload and get a redbox it will still update with new changes?

[gaearon]

Ah, good call. My goal was to make sure RedBox doesn't get into the production build but I can put some more conditionals to ensure that.

[ericclemmons]

👍 from me. I agree that this doesn't belong in these examples, since it's not directly-related to Redux.

[ghost]

Sounds like a good move to me

[dlmanning]

This is what we've drifted towards as our project has gotten larger. Component HMR is just too finicky to maintain, and all the state we care about preserving is in the store

[frontsideair]

I like this, it demonstrates there's nothing magic about hmre.

[Phoenixmatrix]

This is 100x better: handles all the common scenarios for 1/10th of the complexity. I'm in love with it.

[briandipalma]

What's wrong with leaving this as an ES2015 import? Is there a technical reason why this must be changed to an inline require?

[justingreenberg]

the loader spec is still a work in progress but webpack v2 understands the draft proposal, which allows inline (dynamic) imports using system loader, ie:

// webpack v2
let render = () => {
  System.import('./containers/App').then(App => {
    const AppContainer = App.default
    ReactDOM.render(
      <Provider store={store}>
        <AppContainer />
      </Provider>,
      rootEl
    )
  })
}

[gaearon]

The reason it’s an inline require is because we want to import the latest version after hot update.
Babel transpiles import to require() so we have no opportunity to require() again.

In Webpack 2, if we disable Babel modules transform, we will be able use a normal import because Webpack will keep it a live binding and update the imported value. Webpack 2 isn’t quite ready yet though so we don’t rush the migration.

[ericclemmons]

Also, import statements have to be top-level, not nested.

[briandipalma]

Thanks @gaearon makes sense.

[gaearon]

@tomduncalf @mgcrea

Please see the discussion: here's how to make it work with React Router.

[stoikerty]

I've been following the discussion about vanilla HMR and decided to use that approach when upgrading to webpack in my universal-dev-toolkit.

I'm using react-router with a separate routes-file and a RouterContainer that serves as the root-component which is reloaded in the app.js-file using the approach in this PR. It all seems to work well for me so far. When I add a new view to the router, everything refreshes accordingly. Perhaps this could serve as a starting point for someone wishing to implement it themselves?

If you want to add Redux into the toolkit, you can wrap the <Provider store={store}> around the RouterContainer like @0x80 did (see previous comment).

[tomduncalf]

Thanks @gaearon for the pointer and @stoikerty for the information. I'm still struggling to get this working - when updating a component, I either get the component doesn't know how to update itself message if I say module.hot.accept('containers/Root'...), or Warning: [react-router] You cannot change <Router routes>; it will be ignored if I say module.hot.accept('routes'...) (which is the component containing the routes).

I suspect that what is happening is that there is no clear "include path" from my component up to the root component due to the way my app is structured, hence the doesn't know how to update message. There is a clear path from the updated component to the Routes component, but whatever reason routes doesn't want to be reloaded.

I need to take a look at my setup in more detail and work out why this might be happening... not a big deal for now anyway, I'm sticking with class components and react-transform for this project :)

[stoikerty]

@tomduncalf If you provide a more comprehensive example where it doesn't work, say in a repo or a gist, I'm willing to have a look for you. It sounds like the component you're trying to update might be outside of the tree that is on the hot-reload path.

[tomduncalf]

Thanks @stoikerty, that is my suspicion too - my structure is a little more complex than the examples, because I have split the application into separate modules (which are in essence individual single page applications, with their own entry point) but with common code for some parts.

I will put together a minimal reproducible example when I have a moment, and it would be great to get your feedback if I am unable to get it working! Thanks

[tanepiper]

👍 from me, the hmre plugin caused me a lot of headaches a couple of weeks back (mostly documented here: https://gist.github.com/tanepiper/ea41aca1dda6473af738). It was a weird issue I never did resolve in the end, except to switch back to vanilla react-transform-hmr which worked out the box as expected.

[dayyeung]

@gaearon I just tried your modified example, RedBox shows correctly when an error is thrown, but after removing the error, the following occurs: Uncaught TypeError: Cannot read property '_currentElement' of null, and hot reloading no longer works. Is it a bug or is it not supported?

edit: It is actually the opposite, simply throwing an error on hot reload will break it.

[gaearon]

@dayletter

Good catch, this indeed happens with stable React. I was testing with the master version of React. I checked 0.15.0-alpha.1 and it doesn’t seem to have this problem. I would imagine that this was fixed with the initial pass on adding error boundaries in https://github.com/facebook/react/pull/5602/files#diff-c807d0f5ebbc12c44d9479dea693bef6R379.

[imranismail]

Tried with alpha react, the same error happens

[gaearon]

Weird. Are you sure it was 0.15 alpha, you installed it in example’s own folder and that you restarted the server?

[0x80]

@sunyang713 It only relies on standard webpack hmr. You can use both the hot middleware and the standard dev server with hmr enabled.

[Dema]

Guys,so what is the most current way to use react hot reload with redux? Do you have any boilerplace project? https://github.com/gaearon/react-transform-boilerplate seems to be kind of deprecated.

[gaearon]

What you see in this PR is what I recommend for Redux apps after React 15 comes out. I hope to work on a more complete solution that would replace RT later.

[burdiyan]

@gaearon Why did you remove NoErrorsPlugin?

[wdhorton]

He's displaying them with react-redbox instead (see line 24). If he continued using the NoErrorsPlugin, Webpack would suppress the errors, and Redbox would just never be rendered anyway.

[gaearon]

No, this is not the reason. Redbox only catches runtime errors, it doesn’t help with syntax errors.
NoErrorsPlugin has upsides and downsides. I’m likely to add it back. Please see gaearon/react-transform-boilerplate#122 for a list of its upsides and downsides.

[wdhorton]

That makes more sense, sorry for the incorrect explanation

[gaearon]

@sunyang713

Dev server vs express middleware is completely irrelevant to hot reloading. The only reason I prefer middleware in new examples is because it's easier to show how to combine it with server rendering than when you have two separate servers. You can use WebpackDevServer absolutely fine with either this setup or React Transform. This choice is completely unrelated.

[silvenon]

I really benefit from React Transform even though I'm using Redux, because lots of times I'm using component state for things that are awkward to keep in the Redux store, like toggling a modal. A modal is an example where React Transform shines, because sometimes you need a few clicks to open a modal again, so without it you would have to make a few clicks every time you save a file.

[zhangmq]

I want to build isomorphic app with react-router async routes, failed with function component too. I found this hmr solution, but it's hard to implement Root for async routes. I write code below, it works fine, but does not look good. need some advise for better solutions.

hot wrapper

const hot = (r, a) => {
  if (module.hot) {
    let Inner = r();

    const wrapped = class Wrapped extends Component {
      constructor(props) {
        super(props);

        a(() => {
          Inner = r();
          this.forceUpdate();
        });
      }

      render() {
        return <Inner {...this.props} />;
      }
    };

    return wrapped;
  }

  return r();
};

routes

{
    path: '/',
    component: hot(
      () => require('./containers/Application/Application'),
      (update) => module.hot.accept('./containers/Application/Application', update)
    ),
    indexRoute: {
      getComponent(nextState, cb) {
        require.ensure([], require => {
          cb(null, hot(
            () => require('./containers/Home/Home'),
            (update) => module.hot.accept('./containers/Home/Home', update)
          ));
        });
      }
    },
    childRoutes: [
      {
        path: 'about',
        getComponent(nextState, cb) {
          require.ensure([], require => {
            cb(null, hot(
              () => require('./containers/About/About'),
              (update) => module.hot.accept('./containers/About/About', update)
            ));
          });
        },
        onEnter: requireLogin
      }
    ]
  }

[crysislinux]

A note:

I think it's better to also wrap the reducer with try catch block to avoid broken HMR.

if (module.hot) {
  // Enable Webpack hot module replacement for reducers
  module.hot.accept('../reducers', () => {
    try {
      const nextRootReducer = require('../reducers').default;
      store.replaceReducer(nextRootReducer);
    } catch (error) {
      /* eslint-disable no-console */
      console.error('error: ', error);
      /* eslint-enable no-console */
    }
  });
}

[gaearon]

Hey everyone!

While I’m still probably going to keep vanilla HMR in Redux examples for simplicity, I just released an alpha of React Hot Loader 3 which handles functional components, needs very little configuration (especially with regards to server rendering or production builds), works great with higher order components (so you can hot reload mapStateToProps) and contains other improvements.

It’s not 100% ready yet but I encourage you to check it out!

[d-xo]

Why use ReactDOM.render() instead of plain render()?

[justingreenberg]

render is being assigned on line 10...

[timdorr]

@gaearon Can we close this out then?

[timdorr]

Superseded by #1883

[Sawtaytoes]

I saw a comment here about react-router being supported only with synchronous routes:
https://github.com/gaearon/react-hot-boilerplate/blob/next/src/App.js

Is it possible it works with async routes now? I moved my code into a separate root.jsx file to match the examples, and it works so long as I swap my <Router /> component for a <div />. This works with async routes in 1.3.0.

Sadly, on-save with the code as-is, it refreshes the page :/.

client.jsx (entry)

import React from 'react'
import { render } from 'react-dom'
import { match } from 'react-router'
import { AppContainer } from 'react-hot-loader'

// Root Component
import Root from 'root'

// Polyfills
import 'react-fastclick'
import 'utilities/polyfills'

// Store and Routes
import { history } from 'utilities/store'
import routes from './routes'

// Router
match({ history, routes }, (error, redirectLocation, renderProps) => {
    if (redirectLocation) {
        window.location = redirectLocation.pathname
    }

    render(
        <AppContainer>
            <Root renderProps={renderProps} />
        </AppContainer>
    , document.getElementById('root'))

    if (module.hot) {
        module.hot.accept('./root', () => {
            const RootContainer = require('./root').default
            render(
                <AppContainer>
                    <RootContainer renderProps={renderProps} />
                </AppContainer>
            , document.getElementById('root'))
        })
    }
})

root.jsx

import React, { Component } from 'react'
import { Provider } from 'react-redux'
import { browserHistory, Router } from 'react-router'

// Store and Routes
import { store } from 'utilities/store'

export default class Root extends Component {
    render() { return (
        <Provider store={store}>
            <Router history={browserHistory} {...this.props.renderProps} />
        </Provider>
    )}
}

Repo w/ Update Branch:
https://github.com/Sawtaytoes/Ghadyani-Framework-Webpack-React-Redux/tree/feature/update-react-hot-loader

Update Commit:
Sawtaytoes/Ghadyani-Framework-old@c675c42