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

Document React Hot Loader 3 #243

Closed
gaearon opened this issue Apr 18, 2016 · 32 comments
Closed

Document React Hot Loader 3 #243

gaearon opened this issue Apr 18, 2016 · 32 comments

Comments

@gaearon
Copy link
Owner

gaearon commented Apr 18, 2016

It features two modes (react-hot-loader/babel and react-hot-loader/webpack), doesn’t hide HMR API (you have to add module.hot.accept call yourself) and has a somewhat different API and setup process.

There’s info in gaearon/react-hot-boilerplate#61 and reduxjs/redux-devtools@64f58b7 but we should create good step-by-step docs and update the website too.

This will take a while because it’s not a priority for me. If you want to contribute, you are very welcome!
Obviously we can’t release 3.0 without any docs.

@gaearon
Copy link
Owner Author

gaearon commented Apr 18, 2016

To give some sense of perspective, if nobody contributes, the stable 3.0 might get out as late as June because I really don’t have the bandwidth to work on a good doc right now. It really is the biggest blocker to getting this version out!

@pablopaul
Copy link

@gaearon What kind of documentation is needed? Maybe you can give a rough documentation outline or so…

@gaearon
Copy link
Owner Author

gaearon commented Apr 19, 2016

We need to update this intro.

The new intro would be slightly longer because there is a bit more setup involved.
I would split it into several sections:

  1. Enabling HMR
    • Create a development Webpack config separate from production one
    • Add HotModuleReplacementPlugin to development Webpack config
    • If you only render on the client, consider using WebpackDevServer
      • Easier to set up
      • Enable hot: true and add its entry points
    • If you use server rendering, consider using Express server + webpack-dev-middleware
      • More work but also more control
      • Show how to add webpack-dev-middleware and its entry point
  2. Using HMR to replace the root component
    • Show how to add module.hot hook to re-render your root component
    • (This will not preserve state but is good enough for some cases!)
  3. Adding React Hot Loader to preserve state
    • If you use Babel:
      • Add react-hot-loader/babel to plugins
    • If you don’t use Babel:
      • Add react-hot-loader/webpack to loaders
    • Add react-hot-loader/patch as the first entry point
    • Change your index.js to render <AppContainer> instead of your root component
    • That’s it!

@hedgerh
Copy link
Collaborator

hedgerh commented Apr 21, 2016

I can help out with the docs. I've been reading up and gathering as much info as I can. I'm about to go through both the old and 3.0 source to try and figure out what's going on under the hood. I'll see if I can get the ball rolling on the Intro, as well.

vcarl added a commit to reactiflux/q-and-a that referenced this issue Apr 21, 2016
@gaearon
Copy link
Owner Author

gaearon commented Apr 22, 2016

Thanks!

@gadicc
Copy link
Collaborator

gadicc commented Apr 23, 2016

Can I suggest we get started on Hackpad first? I think it will get us started quicker, and then we can use something like the Hackpad downloader to download as markdown to commit, moving forward.

I took an initial bash at https://hackpad.com/React-Hot-Loader-v3-CxSDMncWUnb.

@gaearon
Copy link
Owner Author

gaearon commented Apr 23, 2016

@gadicc @hedgerh Can I ask you to work it out between yourselves? I added you both as collaborators so you should be able to work on a single PR together without much hassle. (Up to you if you want to first take it somewhere else, but I would prefer reviewing on GitHub.) Just a reminder that the PR(s) would need to be sent against the next branch. Thanks!

@tsaiDavid
Copy link
Contributor

tsaiDavid commented Apr 23, 2016

@gaearon @gadicc @hedgerh

I've been following everyone's work and gathering info as well, and would be more than happy to help with documentation. Below, I took the work you all have started and simply rearranged the JSX component tags to make the layout a little more familiar for folks new to this.

We may want to clarify a couple of points that could prove to be confusing:

  • RootContainer is simply an example of a named component, users should be able to swap it out for whatever they have already.
  • The same thing applies to the ID of react-root - I can honestly see this point being a bit confusing as well since this requires configuration on the user's part. I'd be happy to help explain some of these bits.
  • Finally, might we look into using something like GitBook as @gaearon had done with the Redux documentation? Setting up RHL3 would definitely take much longer than usual.
import 'react-hot-loader/patch';
import React from 'react';
import { render } from 'react-dom';
import { AppContainer } from 'react-hot-loader';
import RootContainer from './containers/rootContainer.js';

render((
  <AppContainer>
    <RootContainer />
  </AppContainer>
), document.getElementById('react-root'));

if (module.hot) {
  module.hot.accept('./containers/rootContainer.js', () => {
    const NextRootContainer = require('./containers/rootContainer.js');

    render((
      <AppContainer>
        <NextRootContainer />
      </AppContainer>
    ), document.getElementById('react-root'));
  })
}

@gadicc
Copy link
Collaborator

gadicc commented Apr 25, 2016

@gaearon, sure thing, thanks.

@hedgerh, sorry, I probably should have checked with you first on your progress but I just wanted to get the ball rolling. But now we can collaborate together :)

all, we'll track this in #257.

@tsaiDavid, help would be most welcome, thanks! Everything is on github now so see the issue above for more info.

In particular, I'll mention that I don't use or know how to use webpack, so help is very welcome in all webpack related content.

@gadicc gadicc added docs and removed enhancement labels Apr 25, 2016
@gadicc
Copy link
Collaborator

gadicc commented Apr 25, 2016

@tsaiDavid, did you edit your post right like right now? :) Sorry, I only saw it when refreshing the page, I had left the page open and your edit didn't come through. Anyway, as you saw, everything is on GitHub now... your points are all great, would love you to submit it as a PR to next-docs as described in #257.

@gadicc
Copy link
Collaborator

gadicc commented Apr 25, 2016

@hedgerh, sorry, I also didn't notice your other PRs until right now! I'll take a proper look tomorrow (it's night time here now), but maybe anything that's not in gh-pages we can merge into the next-docs branch and keep all in one place tracked in the single #257.

@hedgerh
Copy link
Collaborator

hedgerh commented Apr 25, 2016

Hey hey @gadicc no worries. I've been off all weekend so I'll need a little bit of time to get caught up.

My progress so far was:

  • I prepped a migration guide in docs/README.md
  • Removed all the starter kits bc I don't think they fit within the scope of the project
  • I took a first pass on the intro that I'm not crazy about, but should be a decent start
  • Added a little to the main readme

Totally open to any changes. Just did as much research as I could and piece things together as I understood it.

@tsaiDavid
Copy link
Contributor

tsaiDavid commented Apr 26, 2016

@gadicc - sounds good I'll get right on it! I did expand my original post at some point, yup

@hedgerh happy to look at stuff for you guys, I love writing 👍

@appsforartists
Copy link

In next-docs, Webpack is recommended as a fallback if you aren't already using Babel. Is there a difference in featureset or reliability between the two adaptors, or is it entirely personal preference?

@gadicc
Copy link
Collaborator

gadicc commented Jun 14, 2016

@appsforartists, the webpack loader only registers/patches react components that are exported by the module/file, whereas the babel plugin works on all top level variable assignments. This will be made clearer in the final version of the docs.

@appsforartists
Copy link

@gadicc That was my hunch. Glad it will be documented. 😃

@hedgerh
Copy link
Collaborator

hedgerh commented Aug 10, 2016

Hey @gaearon, just wanted to check in and see what the status on this was. I haven't checked in on it in a couple of months. I will take another look at them, but I think we just need you or someone that knows the lib to review the docs/site and see if anything needs to be added or removed.

Obviously no rush on my end. Just wanted to touch base!

PR for docs: #257
PR for github pages: #260

@wmertens
Copy link

Question about the module accept code, the current examples are not DRY and risk differing setup between first render and hot reload.

Why not take the function that does the hot accept and run it on first render too? Because of import graph analysis?

@janeklb
Copy link

janeklb commented Sep 4, 2016

@wmertens I agree, its a bit wet so I've been doing something like:

// ... all other stuff
import RootElement from './RootElement';

renderWithHotReload(RootElement);

// Hot Module Replacement API
if (module.hot) {
  module.hot.accept('./RootElement', () => {
    const RootElement = require('./RootElement').default;
    renderWithHotReload(RootElement);
  });
}

function renderWithHotReload(RootElement) {
  render(
    <AppContainer>
      <Provider store={store}>
        <RootElement />
      </Provider>
    </AppContainer>,
    document.getElementById('quickpick-root')
  );
}

@wmertens
Copy link

wmertens commented Sep 4, 2016

Nice, I like it! @gaearon something to make official?

BTW, WET stands for We Enjoy Typing 🙂

On Sun, Sep 4, 2016, 5:40 PM Janek Lasocki-Biczysko <
notifications@github.com> wrote:

@wmertens https://github.com/wmertens I agree, its a bit wet so I've
been doing something like:

import RootElement from './RootElement';

renderApp(RootElement);

// Hot Module Replacement API
if (module.hot) {
module.hot.accept('./RootElement', () => {
const RootElement = require('./RootElement').default;
renderApp(RootElement);
});
}

function renderApp(RootElement) {
render(




,
document.getElementById('quickpick-root')
);
}


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#243 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AADWljp8wpQZSaEhJ7p8RosZMmWTHqc1ks5qmuaDgaJpZM4IJyhC
.

@janeklb
Copy link

janeklb commented Sep 4, 2016

@wmaurer never heard that one before -- excellent :)

yusinto added a commit to yusinto/reactStep2 that referenced this issue Oct 12, 2016
…er> according to this gaearon/react-hot-loader#243. TODO: use webpack-dev-middleware instead of webpack-dev-server
@rvaidya
Copy link

rvaidya commented Oct 30, 2016

A small optimization hack - I noticed that there are a bunch of places in my production javascript where my development env folder path is being included due to react-hot-loader.

If you use the Uglify plugin in your webpack production build you can add a global def in the Uglify config to drop this stuff out.

The react-hot-loader code itself could probably be modified to take better advantage of this by wrapping more of the module itself in if(__REACT_HOT_LOADER__) checks.

new webpack.optimize.UglifyJsPlugin({
    compress: {
        warnings: false,
        screw_ie8: true,
        drop_console: true,
        drop_debugger: true,
        dead_code: true,
        global_defs: {
            __REACT_HOT_LOADER__: undefined // eslint-disable-line no-undefined
        }
    }
}),

@calesce
Copy link
Collaborator

calesce commented Oct 30, 2016

@rvaidya Is this the code from the Babel plugin? It should be omitted if you have NODE_ENV=production. See #367.

@rvaidya
Copy link

rvaidya commented Oct 31, 2016

I'm on Windows so I was using DefinePlugin to define NODE_ENV.

Looks like I can achieve what you suggested by using a combination of DefinePlugin and cross-env.

@calesce
Copy link
Collaborator

calesce commented Oct 31, 2016

@rvaidya Ah, forgot about Windows. 😉 Let me know if that doesn't work.

@rvaidya
Copy link

rvaidya commented Oct 31, 2016

Using both cross env and DefinePlugin does work.

You'd think just using cross-env to set the environment variable would be enough, but my output file sizes are larger without the additional DefinePlugin entry setting process.env.

@calesce
Copy link
Collaborator

calesce commented Oct 31, 2016

I believe DefinePlugin is needed for your own client-side checks, but env variables or cross-env for Babel plugins, other loaders and node_modules. I might be wrong on one of those, but yeah we should add those to our docs.

grrowl added a commit to grrowl/webpack.js.org that referenced this issue Nov 6, 2016
This was the only way I could get it to work, migrating from webpack 1 on a fairly simple project.

Also, seems logical since you either need to render the new reference to App, or re-require it on every `render` call 
(see gaearon/react-hot-loader#243 (comment))
@patsissons
Copy link

patsissons commented Nov 18, 2016

I ran into a really challenging issue to diagnose today. I was importing my API surface first (which imports a bunch of stuff in the correct order), then importing the root component explicitly. This ended up creating a dependency graph that HMR could (or would) not handle. In order to resolve the issue, I had to import my API surface as a namespace and access the root component via the import alias.

This doesn't work:

declare let module: any;

import * as React from 'react';
import { render } from 'react-dom';
import { AppContainer } from 'react-hot-loader';

// import framework API surface
import './webrx-react';

// import Root Component
import { AppView } from './Components'; 

const container = document.getElementById('app');
render(<AppContainer><AppView /></AppContainer>, container);

if ((module as any).hot) {
  (module as any).hot.accept('./Components', () => {
    // ...
  }
}

but this does:

declare let module: any;

import * as React from 'react';
import { render } from 'react-dom';
import { AppContainer } from 'react-hot-loader';

// import framework API surface
import * as wxr from './webrx-react';

const container = document.getElementById('app');
render(<AppContainer><wxr.Components.AppView /></AppContainer>, container);

if ((module as any).hot) {
  (module as any).hot.accept('./webrx-react', () => {
    // ...
  }
}

I tried a number of combinations, but ultimately, if your root component has two parents it gets removed from outdatedDependencies and then we end up walking up the chain to module 0 which returns without a result.

I figured it's possible I'm not seeing something, so checking in with you guys.

I should also mention i tried to import the AppView explicitly from its original export and then accept it directly from there, no dice same issue.

import { AppView } from './Components/Common/App/AppView';

(module as any).hot.accept('./Components/Common/App/AppView', () => {
 // ...
}

@tleunen
Copy link

tleunen commented Nov 21, 2016

So I followed the steps to convert a project to the new "3.x" version and the page is realoading entirely every time a change is made. It wasn't the case before. Any idea on what could be the cause to that?

@calesce
Copy link
Collaborator

calesce commented Nov 21, 2016

@tleunen want to open a new issue with a minimal reproduce project? There's a number of possibilities, with the most common being not pointing to RR routes in module.hot.accept.

I'm working on updating the troubleshooting doc with common 3.0-specific problems.

@tleunen
Copy link

tleunen commented Nov 21, 2016

I'm in a fairly big project so I can try to reproduce it in a smaller one but I'm setting the AppContainer with the root component (which contains the redux provider and router).

if (module.hot) {
    module.hot.accept('./root', renderApp);
}

aitherios added a commit to aitherios/webpack.js.org that referenced this issue Dec 8, 2016
The current code doesn't work with `react-hot-loader@3.0.0-beta.6`, following this discussion gaearon/react-hot-loader#243 I could fix my own code with the changes on how the hot module replacement api was called.
@wkwiatek
Copy link
Collaborator

wkwiatek commented Mar 2, 2017

We just merged #260 so I think we can close this general issue for now. If you can see that something in the docs is not right or there is some room for improvements, feel free to open either issue or PR.

Thanks!

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

No branches or pull requests