search

brigand / react-mixin

mixins in react with es6 style classes
MIT License
1.12k stars 47 forks source link

componentWillMount(): Assigning directly to this.state is deprecated #52

Open cpunion opened 7 years ago

cpunion commented 7 years ago

Use react-mixin with react@16.0.0-beta.3 (maybe also in 15.6.x) will output warning:

Warning: XComponent.componentWillMount(): Assigning directly to this.state is deprecated (except inside a component's constructor). Use setState instead.

Seems https://github.com/brigand/react-mixin/blob/master/index.js#L37-L45 calls applyInitialState, and applyInitialState at https://github.com/brigand/react-mixin/blob/master/index.js#L30 calls instance.state = ...

cpunion commented 7 years ago

It disappeared after change https://github.com/brigand/react-mixin/blob/master/index.js#L33 to instance.setState(state);

moriyoshi commented 7 years ago

getInitialState()s are supposed to be called in the constructor of the classe created by createReactClass(), so instance.state needs to be available at the time componentWillMount() gets called in the first place.

Actually, subsequent code in instance.componentWillMount() may rely on the state being already set to the instance, but the change broke the compatibility because calls to setState() are queued and the state is updated asynchronously. Dispatching it by specifying a callback to setState() won't work either since the timing the callback is called is pretty much unpredictable.

As in-mixing happens in an intrusive manner (that is, modifying the original class itself rather than creating a new class), there's no way to correctly update the state other than directly assigning the state to the instance, as opposed to the deprecation warning.

brigand commented 7 years ago

@moriyoshi thanks for looking into this, but I don't plan on making any changes to this package currently. If you want to implement a version of this lib that works better, I'll put a link to it at the top of the readme.

I updated the readme to warn against using mixins.

moriyoshi commented 7 years ago

Ok, then how about simply reverting the change if you have no plan to fix this further?

brigand commented 7 years ago

@moriyoshi the warning is better than broken behavior, so I reverted that commit. Published as 4.0.0.

OEvgeny commented 7 years ago

Maybe reopen the issue then, to indicate that it present.

brigand commented 7 years ago

Sure.