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

"The following modules couldn't be hot updated: (Full reload needed)" - updated to 3.0 #409

Closed
Foxhoundn opened this issue Oct 31, 2016 · 9 comments

Comments

@Foxhoundn
Copy link

If you are reporting a bug or having an issue setting up React Hot Loader, please fill in below. For feature requests, feel free to remove this template entirely.

Description

What you are reporting: No component is being live updated after update to 3.0

Expected behavior

What you think should happen: Components should hot reload

Actual behavior

What actually happens: They don't, instead a message -

The following modules couldn't be hot updated: (Full reload needed)
This is usually because the modules which have changed (and their parents) do not know how to hot reload themselves. See http://webpack.github.io/docs/hot-module-replacement-with-webpack.html for more details.

is shown.

Environment

React Hot Loader version: "react-hot-loader": "^3.0.0-beta.6",

Run these commands in the project folder and fill in their results:

  1. node -v: v4.4.2
  2. npm -v: v3.8.5

Then, specify:

  1. Operating system: MacOS
  2. Browser and version: any

I've only updated the react hot loader, not webpack itself. We still use WP 1.13.0. It was working fine before the update, is there any new configuration needed?

Thanks

@calesce
Copy link
Collaborator

calesce commented Oct 31, 2016

Yeah there's a few changes needed, see here for an upgrade guide.

@Foxhoundn
Copy link
Author

Foxhoundn commented Nov 1, 2016

Thanks @calesce, got it working <3.

One question though, what does "react-hot-loader/babel" do in .babelrc? Because this change broke my acceptance tests:

 ReferenceError: _this8 is not defined
         at World._callee5$ (world.js:251:19)
         at tryCatch (/Users/filipwitosz/Code/Projects/qorus/node_modules/babel-regenerator-runtime/runtime.js:61:40)
         at GeneratorFunctionPrototype.invoke [as _invoke] (/Users/filipwitosz/Code/Projects/qorus/node_modules/babel-regenerator-runtime/runtime.js:329:22)
         at GeneratorFunctionPrototype.prototype.(anonymous function) [as next] (/Users/filipwitosz/Code/Projects/qorus/node_modules/babel-regenerator-runtime/runtime.js:94:21)
         at step (/Users/filipwitosz/Code/Projects/qorus/features/support/world.js:2:366)
         at /Users/filipwitosz/Code/Projects/qorus/features/support/world.js:2:610
         at new Promise (/Users/filipwitosz/Code/Projects/qorus/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:193:7)
         at World.<anonymous> (/Users/filipwitosz/Code/Projects/qorus/features/support/world.js:2:277)
         at World.__keyDown__REACT_HOT_LOADER__ (/Users/filipwitosz/Code/Projects/qorus/features/support/world.js:327:3419)

I was trying to debug this, tho' the problem was gone when I removed it from .babelrc, and hot reloading still works fine.

Thanks!

@calesce
Copy link
Collaborator

calesce commented Nov 1, 2016

what does "react-hot-loader/babel" do in .babelrc?

react-hot-loader/babel is part of what makes RHL 3 work, by adding some code at compile time that makes potential components "visible" to the runtime code that handles updating proxied components. I recommend reading Dan's post for some background, but we also want to document how it works in detail.

The babel plugin also does some magic to make class properties able to be reloaded, because they're otherwise hidden to react-proxy. This is because class properties are defined on the constructor, which you can't update without re-instantiating the component.

ReferenceError: _this8 is not defined

Are you using async class properties? That looks like the same issue as #391, hopefully we'll resolve that soon.

@Foxhoundn
Copy link
Author

@calesce well i will keep using it without the babel plugin and see if something doesn't work.

We don't really use any async class properties afaik, and the only test that failed was one that uses keyDown event dispatching, which is, in fact, an async class property:

keyDown = async (target, key) => {
    const event = this.browser.window.document.createEvent('HTMLEvents');
    event.initEvent('keydown', true, true);
    event.which = key;
    event.keyCode = key;
    const tr = this.browser.window.document.querySelector(target);
    tr && tr.dispatchEvent(event);
  };

Cheers, fox.

@Foxhoundn
Copy link
Author

@calesce I've found one file where we use a async class property, and hot reloading works fine with this component even without the babel plugin.

class Login extends Component {
  static contextTypes = {
    router: PropTypes.object,
  };

  props: {
    location: any,
    sendAuthCredentials: () => Promise<*>,
  };

  onSubmitSuccess = async () => {
    const { router } = this.context;
    const { location } = this.props;

    const nextUrl = location.query.next || '/';
    router.push(nextUrl);
  };

  handleSubmit = (
    { login, password }: { login: string, password: string }
  ): Promise<*> => (
    auth(login, password, this.props.sendAuthCredentials).then(this.onSubmitSuccess)
  );

  render() {
    return (
      <CenterWrapper>
        <h1>Qorus web application</h1>
        <LoginForm onSubmit={this.handleSubmit} />
        <SystemInfo />
      </CenterWrapper>
    );
  }
}

Maybe this will help in debugging the issue.

@calesce
Copy link
Collaborator

calesce commented Nov 1, 2016

Hmmm, does the keyDown one need to be async? Doesn't look like it's using await.

hot reloading works fine with this component even without the babel plugin

You won't need React Hot Loader if your components don't have state or lifecycle hooks. The reason the Login example works without the Babel plugin is that it completely remounts the component using regular Webpack HMR. You can test that by adding componentWillUnmount with a console.log.

Also, you can use the Webpack loader (react-hot-loader/webpack) as an alternative. It's only able to tag exported components, but it also has less bugs right now. 😉

@Ciantic
Copy link

Ciantic commented Nov 4, 2016

I want to add a one thing: There should be a way to detect and guide the users to do the setting up right:

import * as React from "react";
import { render } from "react-dom";
import { App } from "./Containers/App";
import { AppContainer } from "react-hot-loader";

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

render(<AppContainer>
    <App />
</AppContainer>, rootEl);

if (module.hot) {
    module.hot.accept("./Containers/App", () => {
        // If you use Webpack 2 in ES modules mode, you can
        // use <App /> here rather than require() a <NextApp />.
        const NextApp = require("./Containers/App").App;
        render(
            <AppContainer>
                <NextApp />
            </AppContainer>,
            rootEl
        );
    });
}

There is so much new in there.

I've tried react-hot-loader 3 couple of times, but it didn't guide me in anyway so I thought it was not working with my setup yet. It turned out to be that I just needed to throw this new stuff in there.

Ideally it should somehow detect that user is still trying to use the old way:

import * as React from "react";
import { render } from "react-dom";
import { App } from "./Containers/App";

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

render(<AppContainer>
    <App />
</AppContainer>, rootEl);

Which just doesn't work anymore.

@calesce
Copy link
Collaborator

calesce commented Nov 4, 2016

@Ciantic I agree it's confusing, but the plan is to have the quick setup guide (currently on next-docs) on the README.

Also yeah, it would be beneficial to check via the babel plugin/webpack loader whether a module using AppContainer also has a module.hot.accept callback. But it might be tricky because some people might put AppContainer in the "Root" component in a separate module, which is valid but would still trigger that warning. There's probably a few more edge cases like that.

If you want to write up a PR I'd be happy to look at it.

@calesce
Copy link
Collaborator

calesce commented Nov 5, 2016

Going to close this out because the original issue was resolved, and the async issue is tracked in #391.

@calesce calesce closed this as completed Nov 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants