React-hot-loader: Investigate what it would take to support function properties

[gaearon]

e.g.

class App extends Component {
  handleClick = () => {
    this.setState(...)
  }
}

One way to approach this would be to hoist the bodies of the function properties into hidden methods on the prototype with mangled names. This would be unobservable to the user but make them available for react-proxy to see and hot reload.

[nfcampos]

hoist the bodies of the function properties into hidden methods on the prototype with mangled names

can you do that without a babel plugin?

[gaearon]

I don’t think so. But then again, you can’t have class properties without Babel anyway?

[nfcampos]

yeah but then aren't we back to needing to identify classes that look like react components at compile time?

[gaearon]

No, this transform is harmless for any class. I just mean turning

class Stuff {
  doSomething = (e) => {
    this.x(e.stuff())
  }
}

into

class Stuff {
  doSomething = (e) => {
    return this.___doSomethingqwertyu234567(e);
  },
  ___doSomethingqwertyu234567(e) {
    this.x(e.stuff())
  }
}

I’m not sure if there is anything in the spec that would make it a bad idea but it’s worth giving it a shot. The only special case I see so far is arguments object which should still end up being the one in constructor. To be honest we can bail completely when we see arguments used.

[nfcampos]

Hmm, I see, we might hit some edge cases related to the differences between regular functions and arrow functions, so we should bail out on use of super, new.target, and arguments as you said, and we also need to convert arrow functions without curly braces to regular functions with body

class Stuff {
  doSomething = (e) => this.x(e.stuff())
}

class Stuff {
  doSomething = (e) => {
    return this.___doSomethingqwertyu234567(e);
  },
  ___doSomethingqwertyu234567(e) {
    return this.x(e.stuff())
  }
}

so maybe it might be better to translate it to

class Stuff {
  doSomething = (e) => {
    return this.___doSomethingqwertyu234567(e);
  },
  ___doSomethingqwertyu234567(e) {
    return ((e) => this.x(e.stuff()))(e)
  }
}

this way the function would still be an arrow function... on the other hand we're creating a function on every invocation...

[gaearon]

We need to create it once, otherwise it’s slow.

I’m not really sure I see your point about functions without body. We can just convert them to arrow functions without body that call those hidden methods. Or we can bail out because most event handlers are more than one liners, and commonly React code uses those just for event handlers. Handling majority of cases is good enough here.

[nfcampos]

yeah you're right, this doesn't need to work for everything.
I guess I'm just afraid of anything that changes code behaviour between development (where you'd use react hot loader, and class properties would be converted in this fashion) and production where you don't use it, especially if it changes the behaviour in subtle ways, like the differences between an arrow function and a regular function.

[gaearon]

Yeah I get it, but if Babel is able to compile arrow functions correctly, so can we 😄 . Just a matter of having enough tests.

[calesce]

Added by #322, and out in 3.0.0-beta.4, thanks @nfcampos!