Methods closure is lost after hot reload

[MaskaZan]

There are several forms in my project. So I created a generic input change handler:

//Very simplistic example of universal-handler.js
export default context => e => {
   const { name, value } = e.target
   context.setState({
      [name]: value
   })
}

And I use it like this:

import { hot } from 'react-hot-loader'
import handle from './universal-handler'

class SomeForm extends React.Component {
   constructor() {
      super()
      this.state = { login: '' }
      this.handle = handle(this)
   }

   render() {
      return (
         <form>
            <input
               type='text'
               name='login'
               value={this.state.login}
               onChange={this.handle}
            />
         </form>
      )
   }
}

export default hot(module)(SomeForm)

First everything goes well. But after a hot reload, when I start typing to the input, I get an
error:

Uncaught ReferenceError: context is not defined
at eval (eval at __reactstandin__regenerateByEval (VM48923
App.js:NaN), <anonymous>:10:3)
at HTMLUnknownElement.callCallback (react-dom.development.js:100)
at Object.invokeGuardedCallbackDev (react-dom.development.js:138)
at Object.invokeGuardedCallback (react-dom.development.js:187)
at Object.invokeGuardedCallbackAndCatchFirstError
(react-dom.development.js:201)
at executeDispatch (react-dom.development.js:466)
at executeDispatchesInOrder (react-dom.development.js:488)
at executeDispatchesAndRelease (react-dom.development.js:586)
at executeDispatchesAndReleaseTopLevel
(react-dom.development.js:597)
at Array.forEach (<anonymous​>)

Because the context is lost:

(function REACT_HOT_LOADER_SANDBOX () {
          var _this  = this; // common babel transpile
          var _this2 = this; // common babel transpile
          var _this3 = this; // common babel transpile
          return function (e) {
		var _e$target = e.target,
		    name = _e$target.name,
		    value = _e$target.value;

		context.setState(_defineProperty({}, name, value));
	};
          }).call(this)

[theKashey]

Oh crap :( This is something we could not handle and never will be.
Should not be a problems unless you gonna change the universal-handler.

As far as I know - RHL should not change the method, if regeneration fails. Are you sure about "lost"?

[MaskaZan]

@theKashey

As far as I know - RHL should not change the method, if regeneration fails. Are you sure about "lost"?

It is obvious that RHL calls toString of method to generate REACT_HOT_LOADER_SANDBOX and loses closure of the method

Yes, there all right with universal-handler when I add module.hot.accept to constructor:

class SomeForm extends React.Component {
   constructor() {
      super()
      this.state = { login: '' }
      this.handle = handle(this)
		
      if (module.hot) {
         module.hot.accept('./universal-handler', () => {
            this.handle = handle(this)
         })
      }
   }

   //...

By the way, it would be cool to implement the hot method for calling after hot reload of non-component modules:

import handle from './universal-handler'

class SomeForm extends React.Component {
   constructor() {
      super()
      this.state = { login: '' }
      this.handle = handle(this)
   }

   hot() {
      /* required from non-component module ./universal-handler */
      this.handle = handle(this)
   }

   render() {
      return (
         <form>
            <input
               type='text'
               name='login'
               value={this.state.login}
               onChange={this.handle}
            />
         </form>
      )
   }
}

export default hot(module)(SomeForm)

[theKashey]

Yeah, I thought about it many many times, as long sometimes it is the only way to solve some complex cases. And it is a super easy thing to do.

The main concern against - we probably should be as transparent, as we could. But we are not transparent, and, to be honest, RHL is a one big side effect.

@neoziro - what do you thing about componentDidHotUpdate?

[MaskaZan]

@theKashey Thanks | Спасибо за оперативные ответы :)

[MaskaZan]

Ugly working example:

import { hot } from 'react-hot-loader'
import handle from './universal-handler'

class SomeForm extends React.Component {
   constructor() {
      super()
      this.state = { login: '' }
      this.handle = handle(this)

      if (module.hot) {
         module.hot.accept('./universal-handler', () => {
            this.handle = handle(this)
         })
      }
   }

   render() {
      if (module.hot) {
         //recalc every render
         this.handle = handle(this)
      }

      return (
         <form>
            <input
               type='text'
               name='login'
               value={this.state.login}
               onChange={this.handle}
            />
         </form>
      )
   }
}

export default hot(module)(SomeForm)

[theKashey]

Better utilise componentDidUpdate or (even better)componentWillRecieveProps

[MaskaZan]

I got the same error using componentDidUpdate
Isn't componentWillRecieveProps not recomended?

[theKashey]

You will "always" get that error, while you have this.handle = handle(this) in a constructor.
Using componentDidUpdate/componentWillRecieveProps is just a way around, to actually get this method updated.
But will not solve the problem :(

[brainkim]

Probably related: #984

Using lodash functions like throttle, or debounce (and probably any higher-order functions) to set instance methods i.e.

class MyComponent extends React.Component {
   /* other stuff */
  parse = _.debounce(() => { parser.parse(this.props.content) });
}

will throw a ReferenceError in a function called REACT_HOT_LOADER_SANDBOX.

[theKashey]

Ok, let's try to find a way, to detect, and repeat functions like this.
The problem - we need a way to execute an "update" in current class context. Something like execting constructor with overridden this (ie Component.prototype.constructor.call(otherThis). But we cant do it with ES6 classes.

I could see 2 ways:

1. Use something like "sandbox", lots of proxies, to be able to hydrate and rehydrate any function call. Ie being able to call a constructor, get parse out of a new class, and use it in a new function. Probably this is a dead end.
2. Use babel to split constructor into lines, and then being able to rehydrate not the property result, but creation itself.

ie

const constructionLines = () => ({
  data: [],
  push(a){
    this.data.push(a);
    a();
  }
});
class SomeForm extends React.Component {
   constructor() {
      super()
      this._constructionLines =  constructionLines();
      this._constructionLines.push(() => this.state = { login: '' }))
      this._constructionLines.push(() => this.handle = handle(this))

      if (module.hot) {
         module.hot.accept('./universal-handler', () => {
            this.handle = handle(this)
         })
      }
   }

The "problem" - we should not update ALL the properties in constructor, especially state.

Calling for a babel Jedi!

[theKashey]

May be I am should not try to solve this, as we solved this problem before

• before: create a new class, and pick change we want to pick from it
• after: apply contruction function to the current class, and restore changes we dont want to lose.

The second approach sounds much more doable, but still - constructors are not "repeatable"(in ES6), and something have to move everything off constructor, to another function.

The main problem still the same - we have to get string, and execute in the scope of a current component, or HOC will not work.

class SomeForm extends React.Component {
   constructor() {
      super()
      this._constructorBody();
   }
  __constructorBody() { //just extract everything from constructor! (or duplicate)
      this.state = { login: '' }
      this.handle = handle(this);
    }

  }

Still calling for a babel Jedi!

[brainkim]

Is there a specific reason hot-reloading is so broken for these common React use-cases (i.e. setting instance methods with higher-order-functions)? Is there a way to opt out of hot-reloading methods entirely? Considering my (and probably most people‘s) main use-case for hot reloading is to make small style tweaks, I feel it’s so oblivious to cause components to throw random ReferenceErrors just so component methods aren’t stale. I would gladly reload the page to update my component methods in normal development; I’m not trying to demo a hot-reload tool at a React conference. It looks like there‘s a whole class of bugs besides the ones mentioned here (e.g. #969) caused by not copying over the scope of methods. Maybe a solution might be to put the REACT_HOT_LOADER_SANDBOX function in a try catch block and defer to the original method if an error is thrown?

Sorry for the drive-by complaining, I‘ve just found this bug to be so annoying for the past few days. Don’t have to bandwidth to debug/fix this either 😃 so this probably just comes across as entitled gassing. But if this is new behavior introduced by someone trying to optimize components to be even more hot-reload-y I‘d strongly urge y‘all to roll that baby back cuz it‘s unnecessary and broken.

[theKashey]

The problem is that there is no such thing are hot-reloading in javascript, and we have to emulate it.