Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save bvaughn/982ab689a41097237f6e9860db7ca8d6 to your computer and use it in GitHub Desktop.
Save bvaughn/982ab689a41097237f6e9860db7ca8d6 to your computer and use it in GitHub Desktop.
Example for loading new external data in response to updated props
// This is an example of how to fetch external data in response to updated props,
// If you are using an async mechanism that does not support cancellation (e.g. a Promise).
class ExampleComponent extends React.Component {
_currentId = null;
state = {
externalData: null
};
static getDerivedStateFromProps(nextProps, prevState) {
// Store prevId in state so we can compare when props change.
// Clear out previously-loaded data (so we don't render stale stuff).
if (nextProps.id !== prevState.prevId) {
return {
externalData: null,
prevId: nextProps.id
};
}
// No state update necessary
return null;
}
componentDidMount() {
this._loadAsyncData(this.props.id);
}
componentDidUpdate(prevProps, prevState) {
if (this.state.externalData === null) {
this._loadAsyncData(this.props.id);
}
}
componentWillUnmount() {
// Prevent potential setState calls after unmount,
// (Since these trigger DEV warnigs)
_currentId = null;
}
render() {
if (this.state.externalData === null) {
// Render loading state ...
} else {
// Render real UI ...
}
}
_loadAsyncData(id) {
if (id === this._currentId) {
// Data for this id is already loading
}
this._currentId = id;
asyncLoadData(id).then(externalData => {
// Only update state if the Promise that has just resolved,
// Reflects the most recently requested external data.
if (id === this._currentId) {
this.setState({ externalData });
}
});
}
}
@Stephen-ONeil
Copy link

In my own process of moving away from UNSAFE_... methods, I wrote a component using a pattern that is, in practice, identical to this. While it successfully heads off the setState-on-unmounted warning, in my own testing it doesn't do anything about the underlying issue of potential/real memory leaks.

Could someone else following this pattern try putting their component in a situation where it has unmounted before the promise resolves, and then use a heap snapshot to look for instances of the component persisting in memory? Maybe I'm wrong about my component being an "identical" case to this (could be leaky for other reasons), or maybe we're all just suppressing a warning that exists to flag potential memory leaks.

Thanks!

@Stephen-ONeil
Copy link

Stephen-ONeil commented May 28, 2018

To put it another way, I feel like we both effectively re-invented isMounted(), meaning patterns like this have the same pitfalls laid out in the first couple paragraphs of this post.

@franklixuefei
Copy link

Part of the reason of getDerivedStateFromProps() being static is that it enforces that this function stays pure, because a static method has no access to this, thus mutations like this.setState() are not possible.

@khanhlu2013
Copy link

What about if we remove static getDerivedStateFromProps method all together, and update componentDidUpdate as follow:

componentDidUpdate(prevProps){
  if(this.props.id !== prevProps.id){
    this.setState({externalData:null});
    this._loadAsyncData(this.props.id);
  }
}

@akonsu
Copy link

akonsu commented Jul 25, 2018

A bug:

componentDidUpdate(prevProps, prevState) {
    if (prevState.externalData === null) {
      this._loadAsyncData(this.props.id);
    }
  }

must read

componentDidUpdate(prevProps, prevState) {
    if (this.state.externalData === null) {
      this._loadAsyncData(this.props.id);
    }
  }

@Nico142
Copy link

Nico142 commented Aug 23, 2018

I thought the same as @khanhlu2013. Is there a reason why you used getDerivedStateFromProps instead of comparing the IDs (this.props.id !== prevProps.id) in componentDidUpdate?

@tranvansang
Copy link

shouldn't asyncLoadData catch error?

    asyncLoadData(id).then(externalData => {
      // Only update state if the Promise that has just resolved,
      // Reflects the most recently requested external data.
      if (id === this._currentId) {
        this.setState({ externalData });
      }
    });

should be

    asyncLoadData(id).then(externalData => {
      // Only update state if the Promise that has just resolved,
      // Reflects the most recently requested external data.
      if (id === this._currentId) {
        this.setState({ externalData });
      }
    }).catch(() => this._currentId = null);

@kasajian
Copy link

kasajian commented Mar 9, 2019

akonsu's change is required.
inomdzhon's change is required.

If the author isn't going to update the gist, could someone else create a new one, along with a online running example?

@aidanlister
Copy link

@Stephen-ONeil I feel the same way, if (id === this._currentId) { is basically just an isMounted check ...

@tmkasun
Copy link

tmkasun commented Aug 9, 2019

A bug:

componentDidUpdate(prevProps, prevState) {
    if (prevState.externalData === null) {
      this._loadAsyncData(this.props.id);
    }
  }

must read

componentDidUpdate(prevProps, prevState) {
    if (this.state.externalData === null) {
      this._loadAsyncData(this.props.id);
    }
  }

Yes, This will cause the second render to fail. Because prevState is having old data now.

Blog post have the correct code

@bvaughn
Copy link
Author

bvaughn commented Aug 9, 2019

Thanks for pointing out the typo. I often don't notice comments on Gists. I've updated the example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment