Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSS/React Hot Reloading #93

Closed
jlongster opened this issue Jul 22, 2016 · 29 comments
Closed

CSS/React Hot Reloading #93

jlongster opened this issue Jul 22, 2016 · 29 comments

Comments

@jlongster
Copy link

I think one of the best parts of something like this is that typically complex things to setup like hot reloading just work out of the box. It brings those advanced workflows to everyone.

Hot reloading is such a crucial part of my dev experience, and this project could expose it to others. The only problem is that React hot reloading doesn't work all the time, so I don't know if that is too confusing to beginners.

I haven't looked at how this project actually works yet, but I can't think of any reason why this wouldn't work. Just automatically include webpack-hot-middleware in dev mode, add the right entry point, add the right babel transform, etc. It can be completely hidden from the user.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

CSS hot reloading is there.
React will be there is it’s stable enough but it requires work both on webpack and my side.

@jlongster
Copy link
Author

Oh, it's great that webpack's hot reloading infrastructure is already set up.

Yeah, this is purely anecdotal but when I tried switching from v2 to v3 of react-hot-loader it seemed a bit slower to compile my files. I definitely would make sure that this doesn't add significant overhead to the standard workflow of incremental compilation + refresh.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

The nice thing in this project is we get to control the complete experience. We can replace any part if there is a better solution. e.g. I’m hopeful about https://github.com/motion/pundle.

cc @steelbrain

@jlongster
Copy link
Author

Agreed. I think the compilation from babel+react-hot-loader was the slow part though, not webpack. Anyway, I love that this could turn into the de-facto place to benchmark this kind of stuff and establish a standard for the best way for things to work together.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

I love that you love it.

@steelbrain
Copy link

@gaearon There's a 2x speed boost in steelbrain/pundle#43. It also adds support for requiring non-js files (css for example) and custom hot reloading wrappers. It'll be in the next major pundle version, which you can expect in about a week or two

@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

Thanks! @steelbrain Would you be interested in sending a proof of concept PR switching this project to pundle? Doesn’t have to be fully feature complete, just to get a taste for it.

@steelbrain
Copy link

Sure @gaearon! I'll send in a PR soon

@gaearon
Copy link
Contributor

gaearon commented Sep 3, 2016

Closing for now. CSS hot reloading is there, React hot reloading may come one day. 😉

@gaearon gaearon closed this as completed Sep 3, 2016
@asbjornenge
Copy link

What? React hot-reloading is like the main reason I would want (and need) this tool 😲

@viankakrisna
Copy link
Contributor

@gaearon what's the real issue with react hot reloading? I think it's nice to integrate that in CRA. Also, when it's integrated, more people can see and help react-hot-loader smooth out its issue.

@Scimonster
Copy link

It seems it's actually very simple to add HMR for React to your app. See https://medium.com/@sheepsteak/adding-hot-module-reloading-to-create-react-app-e053fadf569d#.fxu5lgi3z. It's literally an extra 10 lines of code.

Before:

ReactDOM.render(
    (
        <Provider store={store}>
            <App />
        </Provider>
    ),
    document.getElementById('root')
);

After:

function render(Component) {
    ReactDOM.render(
        (
            <Provider store={store}>
                <Component />
            </Provider>
        ),
        document.getElementById('root')
    );
}

render(App);

if (module.hot) {
    module.hot.accept('./App', () => {
        const NextApp = require('./App').default;
        render(NextApp);
    });
}

@anomaly44
Copy link

@Scimonster When I use that solution it still refreshes the whole page, so my redux state still gets lost. Did I forget something?

@grantgeorge
Copy link

@anomaly44 I'm having issues once I started using react-router. Similar situation?

@anomaly44
Copy link

@grantgeorge Yes I m using React-router v4, gonna try taking it out tonight if I can find some time and see if it works without.

@grantgeorge
Copy link

grantgeorge commented Feb 23, 2017

@anomaly44 interesting. I had issues w/ react router but worked when I upgraded to v4 (from v3) last night and did some reconfiguration for the upgrade.

In case this helps, here's my setup:

package.json

"react-router-dom": "^4.0.0-beta.6",

index.js

import React from 'react';
import ReactDOM from 'react-dom';

import Routes from './routes';

import './index.css';

const rootEl = document.getElementById('root');

ReactDOM.render(
  <Routes />,
  rootEl
);

if (module.hot) {
  module.hot.accept('./routes', () => {
    const NextApp = require('./routes').default;
    ReactDOM.render(
      <NextApp />,
      rootEl
    );
  });
}

routes.js

const Routes = () => (
  <Router>
    <div>
      <Route exact path="/" component={Home} />
       ...
    </div>
  </Router>
)

export default Routes

@anomaly44
Copy link

anomaly44 commented Feb 23, 2017

I did the same, the only difference being that my in the 'non hot' render is wrapped with a redux provider. It works, I dont get any errors, but the whole page refreshes, so my redux state still gets reset to the initial state. Strange

@anomaly44
Copy link

https://gist.github.com/anomaly44/83c7d7aa0b92997481f50e25800e6f88 Can you spot anything wrong with this? my component is in my App file. Everything works, except that i lose my redux state when making changes.

@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

When you make changes in which files? And what is your configureStore like?

@anomaly44
Copy link

When I edit some of the default text in App.js

added my store.js to the gist: https://gist.github.com/anomaly44/83c7d7aa0b92997481f50e25800e6f88

@gaearon
Copy link
Contributor

gaearon commented Feb 24, 2017

If you look at the code, you are rendering two different things. Why?

// 1
ReactDOM.render(
        <Provider store={store}>
            <Component/>
        </Provider>,

// 2
        ReactDOM.render(
            <NextApp/>,
            document.getElementById('root')
        );

In one case you are rendering App inside a Provider, in other case you don't use Provider. This is why the state gets lost.

You also declared a render() function but you only use it once. Why did you declare it? The purpose of declaring it is so you could reuse it in both places:

const render = (Component) => {
    return ReactDOM.render(
        <Provider store={store}>
            <Component/>
        </Provider>,
        document.getElementById('root')
    );
};

render(App);


if (module.hot) {
    module.hot.accept('./App', () => {
        const NextApp = require('./App').default;
        render(NextApp);
    });
}

I would recommend to get more comfortable with HMR API before using it. You seem to be copy and pasting examples, so you are missing the intention of these lines (and thus missing why the code did not work).

I hope this helps!

@anomaly44
Copy link

sorry for the confusion, the inconsistencies you pointed out were caused by me switching things around and quickly trying some stuff I found googling because I couldnt get it working. The code you posted above is exactly what I tried the first time to get it working. I should have pasted that code instead of the mangled stuff I ended up posting.

Anyway, I added a console.log in the arrow function in the module.hot.accept. When I save a change in App I see the message in my console for half a second before the page refreshes and the state gets lost. Any idea what might be going wrong?

@gaearon
Copy link
Contributor

gaearon commented Feb 25, 2017

Sorry, no idea! If you post a project I could take a look.

@anomaly44
Copy link

While cleaning out my CRA project with the intent to post the project for you. I found out what was causing the problem.

it was this code I used to implement material-ui:

// App.js
import injectTapEventPlugin from 'react-tap-event-plugin';
injectTapEventPlugin();

Moving this code to a separate file as described in this issue solved the problem.

Thanks for the effort guys, much appreciated.

@gaearon
Copy link
Contributor

gaearon commented Feb 27, 2017

Interesting, thanks!

@danielo515
Copy link

Sorry, I'm confused. It's HMR available ? Should I do something extra to activate it?
Many thanks

@gaearon
Copy link
Contributor

gaearon commented May 8, 2017

HMR API is available so you can use module.hot the same way you would normally do with Webpack.

Something like React Hot Loader is not included so you won’t be able to keep the component local state between hot “reloads”.

@danielo515
Copy link

What's the difference ? May be that React Hot Loader is able to keep the state and all the components and the regular HMR api does not? Because before using @Scimonster suggestion the entire page was being reload (like pressing F5 or command +R ) but now using the module.hot stuff at least the browser is not refreshed, which is an improvement.

@gaearon
Copy link
Contributor

gaearon commented May 8, 2017

May be that React Hot Loader is able to keep the state and all the components and the regular HMR api does not?

This is correct.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants