-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Remove componentWillReceiveProps usages in examples #1952
Remove componentWillReceiveProps usages in examples #1952
Conversation
|
Hi @Chaitanya-Deep, welcome to Draft.js and thanks for taking the time to look into this issue! This change looks good to me 👍🏼, sounds good to remove the sketchy |
|
|
||
| componentWillReceiveProps(nextProps) { | ||
| if (nextProps.content !== this.props.content) { | ||
| componentDidUpdate(prevProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth calling out there's an unused prevState second argument here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made the change but is that necessary?
The third snapshot argument is also unused.
And the docs for typical usage of componentDidUpdate don't include it either
https://reactjs.org/docs/react-component.html#componentdidupdate
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@claudiopro is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
niveditc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Requesting changes to split the gulp native update into a separate PR.
niveditc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! :)
facebook-github-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@niveditc has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…1952) Summary: **Summary** Resolves [1940](facebookarchive#1940) Change usage of `componentWillReceiveProps` in Tex examples to `componentDidUpdate` Pull Request resolved: facebookarchive#1952 Reviewed By: claudiopro Differential Revision: D13417818 Pulled By: claudiopro fbshipit-source-id: b13ff3140c3207cddaeb8d98c239f7dfd4b04a47
Summary: **Summary** Resolves [1940](facebookarchive/draft-js#1940) Change usage of `componentWillReceiveProps` in Tex examples to `componentDidUpdate` Pull Request resolved: facebookarchive/draft-js#1952 Reviewed By: claudiopro Differential Revision: D13417818 Pulled By: claudiopro fbshipit-source-id: b13ff3140c3207cddaeb8d98c239f7dfd4b04a47
Summary: **Summary** Resolves [1940](facebookarchive/draft-js#1940) Change usage of `componentWillReceiveProps` in Tex examples to `componentDidUpdate` Pull Request resolved: facebookarchive/draft-js#1952 Reviewed By: claudiopro Differential Revision: D13417818 Pulled By: claudiopro fbshipit-source-id: b13ff3140c3207cddaeb8d98c239f7dfd4b04a47
Summary
Resolves 1940
Change usage of
componentWillReceivePropsin Tex examples tocomponentDidUpdate