Skip to content
This repository has been archived by the owner on Mar 27, 2021. It is now read-only.

Avoid race condition in async sample code #155

Merged
merged 1 commit into from
Jan 22, 2018
Merged

Avoid race condition in async sample code #155

merged 1 commit into from
Jan 22, 2018

Conversation

jez
Copy link
Collaborator

@jez jez commented Jan 19, 2018

r? @michelle

Summary & motivation

Fixes #154.

There are three solutions we can go with here:

  1. check in constructor
  2. check in componentDidMount
  3. check in componentDidMount with a setTimeout

We've chosen to do the second, because

  • It avoids the original race.
  • It avoids recommending setTimeout as the default.
  • It runs in componentDidMount, which is where browser-dependent code should
    live.

The drawbacks are

  • It violates the "no setState in componentDidMount" idiom of React.
  • It delays the time to first paint.

The "no setState" rule is fine; people who care will have ESLint complaint
that they're doing something weird. They can then make the decision to use this
solution or another.

For the others, it avoids the race with the simplest code.

@jez jez assigned jez and michelle and unassigned jez Jan 22, 2018
@michelle
Copy link
Collaborator

lgtm!

@michelle michelle assigned jez and unassigned michelle Jan 22, 2018
@jez jez merged commit 2b15075 into master Jan 22, 2018
@jez jez deleted the jez-set-state branch January 22, 2018 21:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants