buildo / react-placeholder

A React component to easily replicate your page with nice placeholders while the content is loading
MIT License
1.61k stars 97 forks source link

Don't set state to not-ready if the ready prop is true #81

Open michaelmulley opened 5 years ago

michaelmulley commented 5 years ago

I've encountered a bug with the delay prop which would result in the placeholder showing indefinitely, with {ready: true} in props but {ready: false} in state.

I believe the events which lead to this are, when delay is set to n milliseconds, all of the following happen before n milliseconds is up:

  1. ready prop is set to false, a timeout to set ready state is set and the ID stored in this.timeout
  2. Some detail in props.children is changed, cause componentWillReceiveProps to be called again; this causes another timeout to set ready state to be created, and the ID stored in this.timeout is overwritten
  3. ready prop is set to true, which sets state to {ready: true} and which also deletes the timeout set in step 2 (but, crucially, not the timeout set in step 1)
  4. The timeout set in step 1 fires, setting state incorrectly back to {ready: false}

The pull request is one approach among many to fix this. You could also clear any existing timeout in this.timeout before setting a new one, or change the render method to

return (this.props.ready || this.state.ready) ? this.props.children : this.getFiller();

Thanks!