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

Don't use randomly generated IDs in actions #1109

Closed
marcfallows opened this issue May 24, 2017 · 4 comments
Closed

Don't use randomly generated IDs in actions #1109

marcfallows opened this issue May 24, 2017 · 4 comments

Comments

@marcfallows
Copy link
Contributor

In our storybooks we use seedrandom to seed Math.random because we use them for visual regression. That means that the randomly generated IDs can result in duplicate keys:

Randomly generated id: https://github.com/storybooks/storybook/blob/master/addons/actions/src/preview.js#L16

Use of the ID in a key: https://github.com/storybooks/storybook/blob/master/addons/actions/src/components/ActionLogger/index.js#L22

And we all know how React feels about duplicated keys... 😞

Could a uuid be used instead?

@shilman
Copy link
Member

shilman commented May 24, 2017

@marcfallows Seems reasonable to me! Wanna make a quick PR?

@marcfallows
Copy link
Contributor Author

Happy to raise a PR. Do you have a preference:

  • Using node-uuid to generate a uuid
  • Keeping it simple and have a runtime counter

@ndelangen
Copy link
Member

uuid please 👍

@marcfallows
Copy link
Contributor Author

marcfallows commented Jun 23, 2017

Sorry for the delay, finally made a PR for this: #1347

shilman added a commit that referenced this issue Jun 26, 2017
Use uuid for action IDs instead of Math.random (fixes #1109)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants