search

iansinnott / react-string-replace

A simple way to safely do string replacement with React components
MIT License
652 stars 56 forks source link

Correct Method Calls in README #5

Closed gpbmike closed 8 years ago

gpbmike commented 8 years ago
iansinnott commented 8 years ago

Hey @gpbmike, thanks for the pull request. Thanks for this. I totally overlooked the need for keys in the replacement function.

The only thing I would change is to use something besides the match as the key. It's possible for a match to be the same for two replacements, which means both JSX tags would get the same key, which would lead to only one being rendered.

Could you change the key prop to use the index instead of the match? The callback function is passed the index in addition to the match:

(match, i) => <div key={i}>...</div>

Let me know if that doesn't make sense

gpbmike commented 8 years ago

I copied that bit from the example (https://github.com/iansinnott/react-string-replace/blob/master/example/index.js#L22). In my own code I have something more akin to key={${match}-${i}}. I can change it in both places if you'd like.

iansinnott commented 8 years ago

Ah yes, thanks for pointing that out. I just updated the example code to use something like your approach. Yeah I'm happy to merge this PR if you update those keys to use a combination of match and i.

gpbmike commented 8 years ago

Followed your lead from example.

iansinnott commented 8 years ago

Thanks 😄