Skip to content
This repository has been archived by the owner on Apr 9, 2020. It is now read-only.

No hot reloading with componentDidMount as class property #58

Closed
zdavis opened this issue Jan 30, 2016 · 3 comments
Closed

No hot reloading with componentDidMount as class property #58

zdavis opened this issue Jan 30, 2016 · 3 comments

Comments

@zdavis
Copy link

zdavis commented Jan 30, 2016

In the context of a class

export default class Frontend extends Component {

With this, hot reloading works:

componentDidMount() {
  this.props.dispatch(whoami());
}

But with this, no hot reloading happens in the component or any of its child components:

componentDidMount = () => {
  this.props.dispatch(whoami());
};

Based on #51, I'd expect that making a change to componentDidMount would not trigger a hot reload when declared as a class property.

What surprises me is that making a change to the render method in the component—which is not a class property—fails to trigger a hot reload.

I'm able to consistently reproduce the problem with this test component:

import React, { Component } from 'react';

export default class TestComponent extends Component {

  componentDidMount = () => {
    console.log("why can't I have nice things?");
  };

  render() {
    return (
      <div>{'change me for hot reloading'}</div>
    );
  }
}

While this component hot reloads as expected:

import React, { Component } from 'react';

export default class TestComponent extends Component {

  render() {
    return (
      <div>{'change me for hot reloading'}</div>
    );
  }
}

Maybe a bug with my setup, or maybe a bug with react-transform-HMR. If it's the former, sorry to waste your time—if the latter, perhaps this helps.

(Thanks so much for your work on this and related components. My team and I have truly enjoyed working with Redux, etc.)

@satya164
Copy link

Just out of curiosity, why would you need to make componentDidMount a class property?

@gaearon
Copy link
Owner

gaearon commented Jan 30, 2016

We don't currently support property methods: gaearon/react-proxy#8 (comment)
We may eventually solve it so subscribe to gaearon/react-proxy#8 if you are interested.

@gaearon gaearon closed this as completed Jan 30, 2016
@zdavis
Copy link
Author

zdavis commented Feb 1, 2016

@satya164 Mostly out of a desire for consistency. Declaring functions within a component as bound class properties simplifies "this" scoping, and declaring all class methods the same way seemed sensible (http://babeljs.io/blog/2015/06/07/react-on-es6-plus/#arrow-functions). That said, it's clear there's a cost to doing it that way, and it doesn't work for all component method. Live and learn!

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

No branches or pull requests

3 participants