autobind-decorator icon indicating copy to clipboard operation
autobind-decorator copied to clipboard

The autobind doesn't work occasionally

Open sailorT opened this issue 7 years ago • 9 comments

Hi andreypopp, In my react native project, the autobind doesn't work occasionally. I find the root cause is the origin function would be return when its 'get' has been called twice. I think the bind result should be return in this case.
Please help to review this PR. Thanks!

sailorT avatar Jun 27 '17 09:06 sailorT

Take a wild guess... Is

Object.defineProperty(this, key, {
         value: boundFn,
         configurable: true,
         writable: true
});

running async in react native? Or somewhere in the outter get() function may be async? Because the second time you try to access it, it goes to the inner get(), which just returns the bound function.

stevemao avatar Jul 01 '17 04:07 stevemao

I understand your mean, and I couldn't point out the root cause for it. But I think this protect is better. If you can find the root cause, it would be great! BTW, this error would not be happen when the remotely debug opened!

sailorT avatar Jul 04 '17 03:07 sailorT

This fixed the same issue for me.

In my case I'm using relay. Methods would no longer be bound after I performed a mutation that caused the component tree to update cells in a react-native FlatList. (Only on Android, current versions of all modules)

lprhodes avatar Jul 18 '17 12:07 lprhodes

This breaks calling methods on super for me. The method on the current class gets called again recursively instead of the method on super

e.g.

import React, { Component } from 'react'
import { Text, TouchableOpacity } from 'react-native'
import autobind from 'autobind-decorator'

class ParentComponent extends Components {

  @autobind
  myMethod () {
    console.log('Parent')
  }
}


class ChildComponent extends ParentComponent {

  @autobind
  myMethod () {
    console.log('Child')
    super.myMethod()
  }

  render () {
    return (
      <TouchableOpacity onPress={this.myMethod}>
        <Text>Tap Me</Text>
      </TouchableOpacity>
    )
}

export default ChildComponent

lprhodes avatar Jul 19 '17 14:07 lprhodes

In our project we also encountered the issue this PR was made to fix. We applied @autobind to every Class though (would not do that again in future projects as recomended) and the suggested fix in this PR broke our build in a lot of places. It turned out componentWillUnmount was being called in an infinite loop. The following code snippet may not win a beauty contest, but did solve our problem. Perhaps it can be of help to you or others struggling with the same issue.

...
  return {
    configurable: true,
    get() {
      if (definingProperty || this === target.prototype || this.hasOwnProperty(key)
        || typeof fn !== 'function') {
        const lifecycleMethods = [
          'constructor',
          'componentWillMount',
          'componentDidMount',
          'componentWillReceiveProps',
          'shouldComponentUpdate',
          'componentWillUpdate',
          'render',
          'componentDidUpdate',
          'componentWillUnmount',
        ];
        if (!lifecycleMethods.includes(key) && this.hasOwnProperty(key)) {
          return this[key];
        }
        return fn;
      }
      ...
    }
  };

isilher avatar Dec 18 '17 14:12 isilher

Can we create a unit test against this?

stevemao avatar Mar 15 '18 22:03 stevemao

@stevemao Not sure if it is easy to unit test that, I will try to take a look next week

mieszko4 avatar Mar 23 '18 13:03 mieszko4

Just ran into this issue today, so it's still out there.

xanderdeseyn avatar Oct 06 '18 13:10 xanderdeseyn

@N1ghtly please write a failing unit test to illustrate this issue.

stevemao avatar Nov 10 '18 02:11 stevemao