Closed gaearon closed 5 years ago
This looks great! Without the babel
part of the loader, this won't yet work for HOC-wrapped components (because the original isn't exported), right?
Without the babel part of the loader, this won't yet work for HOC-wrapped components (because the original isn't exported), right?
Correct.
React.createElement
will be patched before any user code is ran, preventing issues with code like this https://github.com/reactjs/react-router/blob/master/modules/RouterContext.js#L29 ?
Just to get this straight, react-hot-loader
was superseded by react-transform-hmr
right? Do you want to go back to react-hot-loader
as favorite once v3.0.0
is done?
React.createElement will be patched before any user code is ran, preventing issues with code like this https://github.com/reactjs/react-router/blob/master/modules/RouterContext.js#L29?
Yes (presuming the configuration is correct). In this example I just put react-hot-loader/patch
in entry
as the first item.
Just to get this straight, react-hot-loader was superseded by react-transform-hmr right? Do you want to go back to react-hot-loader as favorite once v3.0.0 is done?
The existing implementations of both React Transform / React Hot Loader are going away. The new implementation will be branded as React Hot Loader 3, but will internally use a combination of their approaches. It will also not be webpack-specific. “Loader” originally meant a Webpack transform but since it’s a project for “hot reloading” I think it’s fine that we expand the scope, and react-hot-loader
will contain opt-in react-hot-loader/webpack
, react-hot-loader/babel
, and potentially other integrations.
Here is an example repo with jspm/SystemJS used in place of webpack: https://github.com/tyscorp/react-hot-boilerplate/tree/jspm
The proxying stuff does not currently work as no exports have __source
attached.
It seems like react-hot-loader/babel
would work fine with jspm/SystemJS for those using plugin-babel.
I did experiment with creating react-hot-loader/systemjs
by patching SystemJS' _export
function with mixed results. I did manage to get ES6 class-based components to successfully hot reload while maintaining state. It looks like the "instantiate" hook (as opposed to "translate") might be more suited for this task but I was unable to write a plugin that even worked using it. Perhaps @guybedford could weigh in on the most appropriate way to write such a plugin?
I added Babel plugin in case you’d like to try it.
I'm getting a _len is not defined
, as described here. Any idea where that might come from?
@0x80 Seems like a Babel bug (https://phabricator.babeljs.io/T7298). I just pushed a workaround in 3.0.0-alpha.9
, please update.
Are there any special steps to get this to work with react-router
? I'm getting Warning: [react-router] You cannot change <Router routes>; it will be ignored
when I update a component
Are there any special steps to get this to work with react-router? I'm getting Warning: [react-router] You cannot change
; it will be ignored when I update a component
Eh, you’re right, I can’t get this working. Looks like it’s time for https://github.com/reactjs/react-router/issues/2182 to gain some momentum!
@nfcampos @gaearon A workaround for ReactRouter is to put the code that stores an instance of your router into a leaf module in your dependency tree. Then when the modules are busted by the hmr process the router instance isn't recreated. I did this quite simply by creating a module allowing you to cache arbitrary functions across module replacements.
acrossReload.js
import {once} from 'lodash'
const acrossReloadStore = {}
export const acrossReload = (name, func) => {
if(! (name in acrossReloadStore)) acrossReloadStore[name] = once(func)
return acrossReloadStore[name]
}
router.js
...
export const router = acrossReload("router", () => createElement(Router, {history}, ...routes))()
I know it looks weird having the this as a function, basically just because I didn't extend the acrossReload function to accept values. But this is easily achievable.
Also, this will not replace update the router, so if you add a route or remove one you will need to refresh. However since this doesn't happen often, it's ok.
@alexisvincent
I’m not sure it would be enough for this particular approach though. React Hot Loader specifically relies on createElement
being called which I don’t think would happen here.
@nfcampos
I found a really hacky workaround. If you just want to play around with this in your project, add this patch to <Router>
component:
Router.prototype.componentWillReceiveProps = function(nextProps) {
let components = [];
function grabComponents(element) {
// This only works for JSX routes, adjust accordingly for plain JS config
if (element.props && element.props.component) {
components.push(element.props.component);
}
if (element.props && element.props.children) {
React.Children.forEach(element.props.children, grabComponents);
}
}
grabComponents(nextProps.routes || nextProps.children);
components.forEach(React.createElement); // force patching
};
This works in my testing. It requires 3.0.0-alpha.11
to work.
@gaearon I don't mean for react hot loader internals, it was meant as a workaround for @nfcampos. Also, this is what I was doing in react-transform. It's possible it will no longer work, but I don't see why not. createElement would be called the first time the router was constructed. But since the router instance doesn't change often. Calling a proxy.update(Comp) on lower components would update things imported by react router. Am I missing something?
@gaearon thanks, i'll try that
@alexisvincent Unlike React Transform, RHL3 doesn’t wrap your components where they are defined, so just re-executing those files won’t do anything. If some files import
ing them call createElement
with their new versions, it’ll be fine, but RR keeps a reference to old versions and won’t createElement
the new route components, so the proxies never get a chance to update.
Oh right. I see the issue. How are you planning to tell components to call createElement once they're been replaced?
I get patch.dev.js?f09d:40 Uncaught TypeError: createProxy is not a function
On this line https://github.com/gaearon/react-hot-loader/pull/240/files#diff-8be7b0cf5cd0de13e69f70bcf3a11143R48
I use webpack-hot-middleware
so my webpack entry looking like
entry: [
'webpack-hot-middleware/client',
'react-hot-loader/patch',
path.join(__dirname, '../client.js'),
],
@alexisvincent
Oh right. I see the issue. How are you planning to tell components to call createElement once they're been replaced?
Normally this just happens as part of the render process. Component tree gets forcefully re-rendered by AppContainer
, and so any component that is currently rendered will update. Those that aren’t rendered right now, will use the new code the next time they are rendered. So it naturally gets eventually consistent. And for things like static properties, HMR is good enough and most of the code directly interfaces with the real class anyway.
@istarkov
I get patch.dev.js?f09d:40 Uncaught TypeError: createProxy is not a function
Any chance you have an old version of react-proxy
? react-hot-loader
requires 3.x
, maybe something’s wrong in your node_modules
. Try removing it and running npm install
again?
@gaearon Ok I see, so essentially a forceUpdate might be worth building in here. At least into the client side implementation.
@gaearon Because my expectation would be immediate consistency. This is essentially why I would use HMR
@alexisvincent
Ok I see, so essentially a forceUpdate might be worth building in here.
It’s right there.
Because my expectation would be immediate consistency. This is essentially why I would use HMR
Sorry, I didn’t explain it clear enough. The currently mounted components are updated immediately. The “lazy” nature refers to components that are not currently mounted. Like when you edit a <Button>
but there is not a single <Button>
on the page. In this case, it will be updated next time it’s rendered, which is indistinguishable from the developer’s point of view from if it was immediately updated. “If a tree falls in a forest...”
Thank you @gaearon
Looks like I forgot to remove hmre
from package.json
and got two different react-proxy
versions.
Even on clean install (rm -rf node_modules && npm install
)
@istarkov Cool, let us know how it goes! I need some more real world usage to catch issues before this goes stable so I really appreciate your help.
@gaearon Oh Brilliant :D
@alexisvincent Yeah, it’s pretty crazy. I was convinced this wouldn’t work the first couple of times I considered it (back in October!) But counter-intuitively, it does 😄 .
The big problem as I use react-router-relay
- hack above removes router warning, but components does not updated. Any thoughts?
hack above for relay router
let Router = require('react-router/lib/Router');
Router.prototype.componentWillReceiveProps = function (nextProps) {
// Blabla as above
};
const { RelayRouter } = require('react-router-relay');
Feel free to dig in 😄 . I haven’t checked why this might be the case. See also the discussion in https://github.com/reactjs/react-router/issues/2182.
@gaearon really impressed man, this is fantastic work. This approach definitely feels clean, if you know what I mean. As a comment on your medium post. Your style and format of writing is great. Presenting the problem, and then following your thought process. Especially valuable was your explanation of failed attempts. I think we need more of this kind of writing generally in the industry.
@nfcampos I added a hacky but harmless workaround for React Router in 3.0.0-alpha.12
so it should just work now. (The warning is still printed but you can ignore it.)
@gaearon it works now! Thanks! I'll let you know if I run into any other issues.
Anyone has made this work with webpack-hot-middleware
? I've just followed the diff to migrate from the hmre
implementation but module.hot.accept
are never called anymore, worked fined before so the webpack side should be ok, any ideas? I've got nothing in my logs.
My current boilerplate is available here https://github.com/mgcrea/react-webpack-factory if you want to try (npm i; npm start
), I usually edit the prop of the Counter
component inside the Navbar
one to test hot reloading.
@mgcrea I did, my entry in webpack was
entry: [
'webpack-hot-middleware/client',
'react-hot-loader/patch',
path.join(__dirname, '../client.js'),
],
@gaearon If I understand correctly a similar hacky workaround as you did for react-router is also required for any other library which renders components that don't output elements in a normal fashion?
For example in my code I'm getting an error on MediaQuery from react-responsive.
If I understand correctly a similar hacky workaround as you did for react-router is also required for any other library which renders components that don't output elements in a normal fashion?
We would very much like to avoid that. The problem with React Router is that it doesn’t really handle receiving new props. Most components should handle this well, as it’s part of React component contract.
For example in my code I'm getting an error on MediaQuery from react-responsive.
What kind of error? Can you show a usage example that reproduces it?
If anyone else found some of your components unmounted and mounted again instead of re-rendered in place, here's my hint: You always need to export your Component somewhere. This is because React-Hot-Reload can only hook itself to exported stuff (via webpack). It does not actively search your code for Components.
Yeah. This is what react-hot-loader/babel
solves.
I introduced RHL3 to my project with react-router and works perfect excluding one situation. It doesn't update component from IndexRoute. I mean when I update AccountOverview component, I can't see changes without manual reload. Below content of my App.js file.
import React, { Component } from 'react';
import { Router, Route, IndexRoute, IndexRedirect, browserHistory } from 'react-router';
import { syncHistoryWithStore, routerReducer } from 'react-router-redux';
import UserPanel from '../pages/user-panel/UserPanel.js';
import AccountOverview from './../pages/user-panel/account-overview/AccountOverview.js';
import EditProfile from './../pages/user-panel/edit-profile/EditProfile.js';
import Login from '../pages/login/Login.js';
const store = createStore(
combineReducers({
ui,
routing: routerReducer,
})
);
const history = syncHistoryWithStore(browserHistory, store);
const MainApp = ({children}) => (
<div>
{children}
</div>
);
const App = (props) => (
<Provider store={store}>
<Router history={history} onUpdate={() => window.scrollTo(0, 0) }>
<Route path="/" component={MainApp}>
<Route component={UserPanel}>
<IndexRoute component={AccountOverview} />
<Route path="edit-profile" component={EditProfile} />
</Route>
<Route path="login" component={Login} />
</Route>
</Router>
</Provider>
);
export default App;
As a work around I replaced this line
<IndexRoute component={AccountOverview} />
with these
<IndexRedirect to="/account-overview" />
<Route path="account-overview" component={AccountOverview} />
it's not exactly the same but enough in my case.
Good point, I forgot React Router has many kinds of routes. Would you like to send a PR against the next
branch? https://github.com/gaearon/react-hot-loader/commit/005c32ddbf9929980a315d30691ed6c8f7a8a8b6 should give you an idea of how to do it.
Sure. Done.
@gaearon is it possible to pass props to the app component?
Use case: Do some work before rendering to the dom, work is then used by app:
doSomeWork(...).then((x) => {
ReactDOM.render(<App x={x} />, rootEl)
})
@danilobuerger
Yes, <AppContainer component={App} props={{ x: x }} />
. We’ll be changing this to <AppContainer><App x={x} /></AppContainer>
in the next beta, but you can use this for now.
If i initialize a redux store outside of App
though, will i still get hot loading of reducers?
@danilobuerger You get it by explicitly handling module.hot.accept
for the reducers, like here. So this is unrelated.
I got it working by following the linked commit, but I'm confused on one thing, if I cause a javascript error somewhere, eg. in one of my render() methods, by calling a non-existent function, the hot reloading stops, until I do a full reload (and I am notified of this fact in the console).
I thought one of the additions coming in 3.0 is that hot reloading is able to recover even if there were javascript errors? Is that not implemented yet, or did I misunderstood that feature, or a misconfiguration on my part?
edit: I just read in the first message that error reporting is waiting on facebook/react#6020, is that related?
Yes, it is related.
For what it's worth, I ported the counter example app from the redux-devtools
repo to use RHL3 in combination with commonly used libraries. See https://github.com/rybon/counter-hmr. This repo uses React 15, Redux, React Router, React Redux, React Router Redux, Immutable, Reselect and CSS in JS inline styles.
HMR works fine. Hope this helps someone.
@gaearon For the react router workaround that you posted.
Router.prototype.componentWillReceiveProps = function(nextProps) {
let components = [];
function grabComponents(element) {
// This only works for JSX routes, adjust accordingly for plain JS config
if (element.props && element.props.component) {
components.push(element.props.component);
}
if (element.props && element.props.children) {
React.Children.forEach(element.props.children, grabComponents);
}
}
grabComponents(nextProps.routes || nextProps.children);
components.forEach(React.createElement); // force patching
};
This doesn't work for me. You mention that it only works for JSX routes? I'm using POJO's. Thoughts?
@alexisvincent, I don't use react-router nor have I followed that thread, but, if your routes are in objects, there's a good chance that what I wrote in https://github.com/gaearon/react-hot-loader/issues/259 might help you. You'll need an extra hot.modules.accept(..)
section as described there in the file where you assemble that object. But as said I'm not really sure of the specifics of the routing issue, personally.
POJOs with React Router will work after https://github.com/gaearon/react-hot-loader/pull/253 is merged. Still a hack, and should ideally be solved on RR side, but better than nothing.
A Big Update Is Coming
React Hot Loader 3 is on the horizon, and you can try it today. It fixes some long-standing issues with both React Hot Loader and React Transform, and is intended as a replacement for both.
Some nice things about it:
react-hot-loader/babel
from.babelrc
and instead addreact-hot-loader/webpack
to loaders)The docs are not there yet, but they will be added before the final release. For now, this commit is a good reference to upgrading your project from React Hot Loader 1 to React Hot Loader 3 alpha. Then see another commit as a reference for upgrading from React Hot Loader 3 alpha to React Hot Loader 3 beta.
With lessons learned both from RHL and RT, here is a demo of a unified approach.
This is really undocumented for now, and we might change API later, so feel free to play with it at your own risk. 😉
react-hot-loader/webpack
is intended to be optional. We will provide a complementaryreact-hot-loader/babel
that detects unexported components as well. You will be able to use either, depending on whether you already use Babel or not.Known Issues
AppContainer
crashes (https://github.com/gaearon/react-hot-loader/issues/283). Until fixed, a workaround is to render the app directly on the server instead.RedboxReact
sometimes fails to display an error message due to https://github.com/stacktracejs/stackframe/issues/11.React.createFactory
orReact.DOM.*
factories. It works either with JSX or withReact.createElement
. (https://github.com/gaearon/react-hot-loader/issues/277)