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

Hot reloading fails for components which have a class property render method #924

Closed
jackwilsdon opened this issue Apr 2, 2018 · 11 comments
Assignees

Comments

@jackwilsdon
Copy link

jackwilsdon commented Apr 2, 2018

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:

Hot reloading fails for components which have a class property render method.

import React from 'react';

export default class Example extends React.PureComponent {
  render = () => 'Hello, example!';
}

Expected behavior

What you think should happen:

Hot reloading should work correctly for components which have a class property render method.

Actual behavior

What actually happens:

A debug message is logged in the browser console (with logLevel set to debug) and the component does not reload.

React-hot-loader: reconcilation failed due to error 
TypeError: CurrentComponent.prototype.render is undefined
Stack trace:
hotComponentRender@http://localhost:3000/static/js/bundle.js:29410:7
render@http://localhost:3000/static/js/bundle.js:29859:43
hotReplacementRender@http://localhost:3000/static/js/bundle.js:29965:56
next@http://localhost:3000/static/js/bundle.js:29986:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:30016:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:29970:3
next@http://localhost:3000/static/js/bundle.js:29986:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:30016:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:29970:3
next@http://localhost:3000/static/js/bundle.js:29986:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:30016:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:29970:3
hotReplacementRender$1@http://localhost:3000/static/js/bundle.js:30037:5
reconcileHotReplacement@http://localhost:3000/static/js/bundle.js:30046:10
renderReconciler@http://localhost:3000/static/js/bundle.js:30060:7
asyncReconciledRender@http://localhost:3000/static/js/bundle.js:30068:3
proxiedRender@http://localhost:3000/static/js/bundle.js:29417:5
finishClassComponent@http://localhost:3000/static/js/bundle.js:14844:24
updateClassComponent@http://localhost:3000/static/js/bundle.js:14812:12
beginWork@http://localhost:3000/static/js/bundle.js:15433:16
performUnitOfWork@http://localhost:3000/static/js/bundle.js:18245:16
workLoop@http://localhost:3000/static/js/bundle.js:18268:26
renderRoot@http://localhost:3000/static/js/bundle.js:18299:9
performWorkOnRoot@http://localhost:3000/static/js/bundle.js:18855:24
performWork@http://localhost:3000/static/js/bundle.js:18776:9
performSyncWork@http://localhost:3000/static/js/bundle.js:18753:5
requestWork@http://localhost:3000/static/js/bundle.js:18653:7
scheduleWorkImpl@http://localhost:3000/static/js/bundle.js:18528:13
scheduleWork@http://localhost:3000/static/js/bundle.js:18488:12
enqueueForceUpdate@http://localhost:3000/static/js/bundle.js:13165:7
./node_modules/react/cjs/react.development.js/Component.prototype.forceUpdate@http://localhost:3000/static/js/bundle.js:30618:3
updateInstances/module.updateTimeout</<@http://localhost:3000/static/js/bundle.js:30229:16
updateInstances/module.updateTimeout<@http://localhost:3000/static/js/bundle.js:30228:7
hotComponentRender@http://localhost:3000/static/js/bundle.js:54668:7
render@http://localhost:3000/static/js/bundle.js:55117:43
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55223:56
next@http://localhost:3000/static/js/bundle.js:55244:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:55274:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55228:3
next@http://localhost:3000/static/js/bundle.js:55244:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:55274:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55228:3
next@http://localhost:3000/static/js/bundle.js:55244:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:55274:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55228:3
next@http://localhost:3000/static/js/bundle.js:55244:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:55274:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55228:3
next@http://localhost:3000/static/js/bundle.js:55244:7
hotReplacementRender/<@http://localhost:3000/static/js/bundle.js:55274:9
hotReplacementRender@http://localhost:3000/static/js/bundle.js:55228:3
hotReplacementRender$1@http://localhost:3000/static/js/bundle.js:55295:5
reconcileHotReplacement@http://localhost:3000/static/js/bundle.js:55304:10
renderReconciler@http://localhost:3000/static/js/bundle.js:55318:7
asyncReconciledRender@http://localhost:3000/static/js/bundle.js:55326:3
proxiedRender@http://localhost:3000/static/js/bundle.js:54675:5
finishClassComponent@http://localhost:3000/static/js/bundle.js:40102:24
updateClassComponent@http://localhost:3000/static/js/bundle.js:40070:12
beginWork@http://localhost:3000/static/js/bundle.js:40691:16
performUnitOfWork@http://localhost:3000/static/js/bundle.js:43503:16
workLoop@http://localhost:3000/static/js/bundle.js:43526:26
renderRoot@http://localhost:3000/static/js/bundle.js:43557:9
performWorkOnRoot@http://localhost:3000/static/js/bundle.js:44113:24
performWork@http://localhost:3000/static/js/bundle.js:44034:9
performSyncWork@http://localhost:3000/static/js/bundle.js:44011:5
requestWork@http://localhost:3000/static/js/bundle.js:43911:7
scheduleWorkImpl@http://localhost:3000/static/js/bundle.js:43786:13
scheduleWork@http://localhost:3000/static/js/bundle.js:43746:12
enqueueForceUpdate@http://localhost:3000/static/js/bundle.js:38423:7
./node_modules/react/cjs/react.development.js/Component.prototype.forceUpdate@http://localhost:3000/static/js/bundle.js:58705:3
updateInstances/module.updateTimeout</<@http://localhost:3000/static/js/bundle.js:55487:16
updateInstances/module.updateTimeout<@http://localhost:3000/static/js/bundle.js:55486:7
react-hot-loader.development.js:88

Environment

React Hot Loader version: 4.0.1

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

  1. node -v: v8.10.0
  2. npm -v: v5.6.0

Then, specify:

  1. Operating system: Ubuntu 17.04
  2. Browser and version: Firefox 59.0.2 (64-bit)

Reproducible Demo

https://github.com/jackwilsdon/react-hot-loader-class-properties

Steps to reproduce

  1. Start the server with npm run start or yarn start.
  2. Visit the application in your browser and note it says "Hello, example!".
  3. Change src/components/Example.js to say something else.
  4. Note that the component does not re-render and that an error is logged in the console.
@jackwilsdon
Copy link
Author

I did a bit of investigative work into this, and it seems that this is due to how class properties work and how we access the render method of a component on hot reload.

Babel converts class properties to look something like this;

function Example() {
  this.render = function() {
     return "Hello, example!";
  };
}

Which does not put the method on the prototype. The issue is that we try and call the render method on the prototype here;

// and can be null or some other falsy value.
if (has.call(this, CACHED_RESULT)) {
result = this[CACHED_RESULT]
delete this[CACHED_RESULT]
} else if (isFunctionalComponent) {
result = CurrentComponent(this.props, this.context)
} else {
result = CurrentComponent.prototype.render.call(this)
}

I'm not sure if there is a solution to this or whether we should just add a warning about it, as it doesn't seem like you can re-bind the context of an arrow function.

@theKashey
Copy link
Collaborator

Yeah, it is better to codemod this code, as long we could not distinguish a "local prop" render method and "render method we got from the prototype". Just because we are using render as a descriptor.
To many of them :)

@jackwilsdon
Copy link
Author

jackwilsdon commented Apr 24, 2018

I guess there's no solution for this then. Cheers 👍

@theKashey
Copy link
Collaborator

Sorry about it. Might be solved with #840

@gregberge
Copy link
Collaborator

@jackwilsdon sorry, a wrong click!

@jackwilsdon jackwilsdon changed the title Hot reloading fails for components which have a class property render method Hot reloading fails for components which have an arrow function render method Apr 24, 2018
@jackwilsdon jackwilsdon changed the title Hot reloading fails for components which have an arrow function render method Hot reloading fails for components which have a class property render method Apr 24, 2018
@Deadly0
Copy link

Deadly0 commented May 12, 2018

There is no solution for this issue at all? react-hot-loader not working for classes and it's no way to fix it?

@jackwilsdon
Copy link
Author

@neoziro Should this be re-opened if it was a mis-click? 😛

@jackwilsdon
Copy link
Author

@Deadly0 it works fine for classes, it just fails with class property render methods;

class MyComponent extends React.Component {
  render() {
    return "This will hot reload fine!";
  }
}

class MyOtherComponent extends React.Component {
  render = function() {
    return "This will fail to hot reload!";
  }
}

@gregberge gregberge reopened this May 12, 2018
@theKashey
Copy link
Collaborator

Ok, I just start typing a long story why I have to idea how to solve this problem, and thus could not.
And then I got an idea how I could 🤷‍♂️

@theKashey theKashey self-assigned this May 13, 2018
@theKashey
Copy link
Collaborator

So the main thing here - there were no issues with the provided code. Only for the nested components, and even they possible were able to hot-reload themselves.
I had to spend some time to get red tests to fix them. Anyway - PR is opened.

theKashey added a commit that referenced this issue May 13, 2018
theKashey added a commit that referenced this issue May 13, 2018
theKashey added a commit that referenced this issue May 13, 2018
fix: render as a class prop. Fixes #924
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

4 participants