Race condition on <Can />

[dfee]

Well, it might not actually be a race condition, as the intended action loses every time, but:

When the component's componentWillReceiveProps is called with a new Ability instance, a setState is called, and then the component's connectToAbility is called which ultimately fires off this.recheck().

casl/packages/casl-react/src/Can.js

Lines 54 to 61 in 774457a

	componentWillReceiveProps(props) {
	if (props.ability && this.state.ability !== props.ability) {
	this.setState({ ability: props.ability });
	this.connectToAbility(props.ability);
	} else {
	this.recheck(props);
	}
	}

That method (this.recheck) calls this.check and uses this.props as params and this.state.ability as the ability. However, the call stack is still nested under componentWillReceiveProps, so the props we're rechecking against are the old props, and the state is most likely not the (now updated) state (see: https://reactjs.org/docs/react-component.html#setstate)!

The result is that the render method uses this.state.allowed which was (likely) produced erroneously as described above.

Therefore a good solution to this problem is to move the call to connectToAbility into the setState callback:

  componentWillReceiveProps(props) {
    if (props.ability && this.state.ability !== props.ability) {
      this.setState({ ability: props.ability }, () => {
        this.connectToAbility(props.ability);  // or this.state.ability
      });
    } else {
      this.recheck(props);
    }
  }

[dfee]

I tried to fork, clone, and test the package before making any changes (and a pull request), but I can't figure out how to get the tests to run.

[dfee]

Alright, figured out how to get it to run, and submitted a PR #129

[stalniy]

I see. Thanks for the issue!

[stalniy]

published in @casl/[email protected]

[dfee]

Thanks for the quick update!