Closed Bug 1431566 Opened 2 years ago Closed 2 years ago

avoid setState race in TopSites and use eslint to prevent similar

Categories

(Firefox :: New Tab Page, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
Firefox 60
Iteration:
60.1 - Jan 29
Tracking Status
firefox60 --- fixed

People

(Reporter: dmose, Assigned: dmose)

References

()

Details

Referencing this.state directly in a call to setState is racy.  I'll attach a patch the fixes the instance we've got of this, and prevents it in the future by turning on an eslint rule.
Component: Activity Stream → Activity Streams: Newtab
Product: Firefox for Android → Firefox
Version: unspecified → Trunk
I've also filed https://github.com/yannickcr/eslint-plugin-react/issues/1648 to allow eslint to also detect uses of this.state in the same synchronous scope after calling this.setState (where either the callback or componentDidUpdate should have been used instead).  If/when that gets implemented, I'll file a separate bug to use that to clean those races out of our codebase as well.
Commits pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/713db8e350135f25f23758ce0c86078c9cc1ce46
fix(TopSites) avoid setState race in TopSites and use eslint to prevent similar, bug 1431566

https://github.com/mozilla/activity-stream/commit/e1abbdfe10e76ed6766979b4065b4928f77d619a
Merge pull request #3949 from dmose/setState-race-bug-1431566

fix(TopSites) avoid setState race in TopSites , use eslint to prevent similar, fixes bug 1431566
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Blocks: 1431945
https://hg.mozilla.org/mozilla-central/rev/549d78378587
Iteration: --- → 60.1 - Jan 29
Target Milestone: --- → Firefox 60
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.