Update to use getDerivedStateFromProps #29
Conversation
| @@ -16,15 +16,22 @@ const textPropTypes = Text.propTypes || {} | |||
| const { OS } = Platform | |||
|
|
|||
| class Hyperlink extends Component { | |||
| static getDerivedStateFromProps(nextProps, prevState) { | |||
| if (nextProps.linkify !== prevState.linkifyIt) { | |||
There was a problem hiding this comment.
I am not sure if this will require using a lodash.isEqual, or something of the sort, but will help as right now every time the component receives props a new instance of linkifyIt is being created
There was a problem hiding this comment.
Testing this locally now, it seems this change has a problem if you do not pass in the linkify prop from outside. That will trigger a getDerivedStateFromProps where nextProps.linkify is undefined, and prevState.linkifyIt is the default value from the constructor (require('linkify-it')()), and thereby unset the state variable.
|
Some news about this PR ? React warn us about deprecated usage, this PR seems to be ok, why this is unmerged ? |
|
Also getting this warning. Might have to patchPackage this until this is merged but would be great to get it in @obipawan |
|
@obipawan Can this PR be taken up & merged? Thanks! Seems like a reasonably sized PR ;-) |
|
The PR has a conflict that needs to be solved, but it would be great to get this in now that the latest version of react native gives a warning for using componentWillReceiveProps. |
|
Resolved the conflicts, should be good to go 👍 |
|
There actually were still a few issues with this PR's changes; running |
| @@ -52,10 +59,10 @@ class Hyperlink extends Component { | |||
| return typeof component.props.children !== 'string' | |||
| } | |||
|
|
|||
| linkify(component){ | |||
| linkify = (component) => { | |||
There was a problem hiding this comment.
The babel build script failed due to this change; transforming this new class property syntax requires the babel plugin proposal-class-properties, which requires Babel 7
|
@ShotSkydiver Yeah, that makes sense. Just merged your PR into my fork |
|
Hi @chrismllr @ShotSkydiver |
|
Available with v0.0.17 👍 |
Also stores the linkifyit instance within state, instead of on the Class.
Resolves issue #27