Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix testimonials of key prop error #71

Merged
merged 3 commits into from
May 5, 2018

Conversation

sakit0
Copy link
Contributor

@sakit0 sakit0 commented Apr 12, 2018

I fixed Product page key prop error.
2018-04-12 19 23 25

@erquhart
Copy link
Contributor

Thanks for the PR!

The commits here don't seem to address the issue, did you push the right branch?

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 12, 2018

@ekoeryanto
sorry.
I forgot push.

@ekoeryanto
Copy link
Contributor

@mki-skt, anything I can help?

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 12, 2018

@ekoeryanto
sorry.
I made a mistake.

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 12, 2018

@erquhart
Thanks🙌

@AustinGreen
Copy link
Contributor

@mki-skt although there are exceptions to the rule, it is generally considered an anti-pattern to use index as keys (read more here: https://reactjs.org/docs/lists-and-keys.html#keys). I like the uuid package for generating unique identifiers

import { v4 } from 'uuid';

const Testimonials = ({ testimonials }) => (
  <div>
    {testimonials.map((testimonial,index) => (
      <article key={v4()} className="message">
        <div className="message-body">
          {testimonial.quote}
          <br />
          <cite> – {testimonial.author}</cite>
        </div>
      </article>
    ))}
  </div>
)

@erquhart
Copy link
Contributor

erquhart commented Apr 13, 2018

My only reasoning here is that this isn't an app, so those maps are static and won't ever change. But yeah, agreed on the general and proper route being unique id's.

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 14, 2018

@AustinGreen
Thank you🙌
It is certainly best to unique a key,But I did not know uuid, I felt it was bored to generate,
I learned uuid and studied!👏

I will fix it!

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 14, 2018

I fixed it.
2ef0326
272f15d

@sakit0
Copy link
Contributor Author

sakit0 commented Apr 16, 2018

@erquhart @AustinGreen
As a result of various experiments, when uuid is used, uuid changes and re-renders when you state change it, what do you think?

@erquhart
Copy link
Contributor

If you're going to use uuid to generate keys, you need those keys to not change - generating a new key on every render has the same net effect as using indexes. That being the case, you either need to add the id to the data before it's passed in as a component prop, or else use a stateful component and create the key in a lifecycle method other than render, or in the constructor.

@AustinGreen AustinGreen merged commit da491f0 into decaporg:master May 5, 2018
csw1995 pushed a commit to csw1995/unused-blog that referenced this pull request Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants