Feature request: warn when this.state is touched directly rather than with setState

[dmose]

I've just stumbled across some code written which touches this.state directly and then forces an update. The danger in this pattern is that appears to work, it still leaves the code vulnerable to other callers of setState doing the right thing, being queued up, and then overwriting the hand-touched state, if I'm understanding the red box on http://facebook.github.io/react/docs/component-api.html correctly.

Any chance if, in (at least) the dev version, this.state could be guarded with a JS setter that would print a warning?

[cirocosta]

Hi @dmose ! This seems like a great thing! I've made PR trying to address it (see #2283). I'd appreciate to hear what you think!

[iansu]

Hi, I'd like to take a look at this.

[kaushik94]

Hey, this bug seems to be open since long. I got some free time, can I try this?

[aweary]

Hey @kaushik94! Feel free to give it a shot! There was a previous PR open for this that you should check out: #2283. Read through the comments to get a good idea of how to approach this 👍 Feel free to ping me if you have any questions!

[iansu]

Hi. I have an open pull request (#8265) that addresses this issue in the way described by @sebmarkbage in #2283. It's just waiting on a code review.

[aweary]

Thanks for reminding me @iansu, I missed it as the PR didn't link to this issue (I'll add a mention to the top comment for you). I'll try and make sure it gets reviewed soon!

[kaushik94]

@aweary I just skimmed through @iansu 's PR and it looks great to me, I don't have to work on this I guess. @aweary can you point me to any issue that I can help with ? All the easy to solve issues has someone already working. Thanks :)

[bvaughn]

React currently warns if this.state is replaced- because it's not the recommended way to update state and may mask potential bugs. However we still support it (and will likely continue to support it).

We do not detect or warn about direct modifications to state properties though (eg this.state.foo = 'bar'). I think this is okay. I don't think it would be worth the additional cost of cloning and doing a shallow comparison of state after each lifecycle method just to warn about something that we actually support (even though it's not recommended).

So I'm going to close this issue. 😄