-1

I'm really confused as to why the "this" inside the class methods refers to window object instead of class object, I'm developing within the node environment and am exporting the class to be used within an React component, i've created the react app using create-react-app

here's the class code: it takes a dom element and applies a shake animation to it.

// make into a class/instance for shake animation on individual cards.
// that individual card will hold reference to obj instance
class ShakeElement{
    constructor(div){
        this.elementRef = div;
        this.deg = 0;
        this.loop = null; // setInterval ref
        this.shakeAnim = null;
        this.clearShakeAnimAfterInterval = null; // setTimeout ref
    }
    // var deg = rotated ? 0 : 66;

    alterAngleOfElement(){
        console.log("angle running for " + this.elementRef);
        this.deg+=1;
        this.elementRef.style.webkitTransform = 'rotate('+this.deg+'deg)';
        this.elementRef.style.mozTransform    = 'rotate('+this.deg+'deg)';
        this.elementRef.style.msTransform     = 'rotate('+this.deg+'deg)';
        this.elementRef.style.oTransform      = 'rotate('+this.deg+'deg)';
        this.elementRef.style.transform       = 'rotate('+Math.cos(this.deg * 1000)+'deg)';
        if(this.deg >= 360){
            this.deg = 0;
        }
    }

    shakeEleCycle() {
        var self = this;
        console.log(this);
        console.log(this.alterAngleOfElement);
        this.shakeAnim = window.setInterval(this.alterAngleOfElement, 20);
        this.clearShakeAnimAfterInterval = window.setTimeout(() => {
            window.clearInterval(this.shakeAnim);
        }, 500);
        console.log("id to clear" + window.shakeAnim);
    }

    start(){
        console.log("STARTING");
        this.loop = window.setInterval(this.shakeEleCycle, 1000);
    }

    stop(){
        console.log("STOPPING");
        // clean up of timers
        window.clearInterval(this.loop);
        window.clearInterval(this.shakeAnim);
        window.clearTimeout(this.clearShakeAnimAfterInterval);
    }
}

module.exports = ShakeElement;

here's the react component where i instantiate the object and call the methods (not sure if that part is relevant)

class Card extends Component {


    // isSpinning: false    -- state

    constructor(props) {
        super(props);
        this.spinCard = this.spinCard.bind(this);
        this.shakeElementHelper = null;
    }

    spinCard(nodeId){ // wobble card event
        this.shakeElementHelper = new ShakeElement(this["card"+ nodeId]);
        console.log(this.shakeElementHelper);
        this.shakeElementHelper.start();
    }


    componentDidUpdate(prevProps, prevState){
        // nest functions in here with specific purposes to run different logic, break it up
        // this.setState({isSpinning: true});
        if(this.shakeElementHelper != null){
            this.shakeElementHelper.stop();
        }

        if(this.props.isFan === true)
            this.spinCard(this.props.id); // or id of redux item object
    }

    // componentWillMount

    componentWillUnmount(){
        // if(this.spinTimerId != null){
        //     window.clearInterval(this.spinTimerId);
        // }
        if(this.shakeElementHelper != null)
            this.shakeElementHelper.stop();
    }

    render() {
        return (
            <div className="card" ref={(ele) => {
                this["card" + this.props.id] = ReactDOM.findDOMNode(ele);
            }} style={this.props.styles}>
                <CardHintInformation gender={this.props.gender} school={this.props.school}/>
                <CardHeader name={this.props.name} surname={this.props.surname} profession={this.props.profession}/>
                <CardSectionsContainer bio={this.props.bio} tags={this.props.tags} links={this.props.links}/>
            </div>
        );
    }
}

any help greatly appreciated, thanks

Zoolu
  • 39
  • 7
  • 2
    When the `alterAngleOfElement` timer fires, the value of `this` won't be bound, so `this` is the global object. The way `this` works in JavaScript is a lot different than other superficially similar languages. – Pointy Apr 15 '18 at 16:01

2 Answers2

2

In

this.shakeAnim = window.setInterval(this.alterAngleOfElement, 20);

You are passing the function address of alterAngleOfElement, but it is not bound to the this of the instantiated object.

In your constructor of ShakeElement, you should:

this.alterAngleOfElement = this.alterAngleOfElement.bind(this)

so that it is bound the instantiated object. You'll see you do this elsewhere.

Also, consider refactoring your clean up code, as there is no guarantee that just because the interval is longer, the cleanup will fire after the animation has completed (due to browser background coalescing).

Dave Meehan
  • 3,133
  • 1
  • 17
  • 24
  • thank you, i updated the code and can see now why it didn't work. in terms of cleaning up all the timers: it it not enough to just clear all the timers that exist? – Zoolu Apr 15 '18 at 19:58
  • 1
    It's probably best if you ask a new question and post your updated code, and see how you are handling cleanup. – Dave Meehan Apr 15 '18 at 21:44
1

What @pointy said is correct,

When the alterAngleOfElement timer fires, the value of this won't be bound, so this is the global object.

Couple of changes are needed here,

shakeEleCycle() {
    var self = this;
    console.log(this);
    console.log(this.alterAngleOfElement);
    this.shakeAnim = window.setInterval(this.alterAngleOfElement, 20);
    this.clearShakeAnimAfterInterval = window.setTimeout(() => {
        window.clearInterval(this.shakeAnim);
    }, 500);
    console.log("id to clear" + window.shakeAnim);
}

Change it to,

shakeEleCycle() {
    var self = this;
    console.log(this);
    console.log(this.alterAngleOfElement);
    this.shakeAnim = window.setInterval(this.alterAngleOfElement.bind(this), 20);
    this.clearShakeAnimAfterInterval = window.setTimeout(() => {
        window.clearInterval(self.shakeAnim);
    }, 500);
    console.log("id to clear" + window.shakeAnim);
}

Now using the self variable that you've declared and the bind function to bind the value of this to the function.

Please read the note from @Dave below,

It's not considered efficient to bind on each call, when it can be done once in the constructor. Functionally this is correct, but sub-optimal. I point this out only because its a bad habit to get into, as repetitive use of bind can hurt render performance in React.

So better to do it once in the constructor rather than do it multiple times in the setInterval

You can read more about how this keyword in JavaScript works here and you can read more about the bind function here.

You should already know the bind function because you are using it here,

this.spinCard = this.spinCard.bind(this);

You can read more about why this is needed in React here.

Abijeet Patro
  • 2,842
  • 4
  • 37
  • 64
  • 1
    It's not considered efficient to bind on each call, when it can be done once in the constructor. Functionally this is correct, but sub-optimal. I point this out only because its a bad habit to get into, as repetitive use of `bind` can hurt render performance in React. – Dave Meehan Apr 15 '18 at 16:13
  • 1
    Agreed, updated as per your comment. – Abijeet Patro Apr 15 '18 at 16:21
  • thank you guys, it now works. cheers. i binded the methods within the class inside the constructor – Zoolu Apr 15 '18 at 19:57