-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Warn if story with a given name already exists #670
Warn if story with a given name already exists #670
Conversation
Right now if you add another story with the same name, the first one is lost. For example: ```js // … storiesOf('Button', module) .add('with text', () => ( // this story will be lost <Button onClick={action('clicked')}>Hello Button</Button> )) .add('with some emoji', () => ( <Button onClick={action('clicked')}>😀 😎 👍 💯</Button> )) .add('with text', () => ( <p> this will silently overwrite the first story </p> )); ```
@@ -11,6 +11,10 @@ export default class StoryStore { | |||
} | |||
|
|||
addStory(kind, name, fn) { | |||
if (this.hasStory(kind, name)) { |
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.
I like to do this check here instead of in the store.
See: https://github.com/evgenykochetkov/react-storybook/blob/01f79bc1dc96a414c89fbcd2b568894f6eebff9b/src/client/preview/client_api.js#L48
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.
Done!
The real one in `src/client/preview/story_store.js` has this method.
@@ -45,6 +45,10 @@ export default class ClientApi { | |||
}); | |||
|
|||
api.add = (storyName, getStory) => { | |||
if (this._storyStore.hasStory(kind, storyName)) { | |||
throw new Error(`Story of "${kind}" named "${storyName}" alredy exists`); |
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.
typo: alredy/already
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! Fixed it.
"alredy" -> "already"
Thank you so much! |
Doesn't it break any existing projects or addons which works with stories dynamically? |
Do you have examples of this? If this is indeed a common thing addons do, We need to think of something else. |
@usulpro Do you think this is a real issue? |
Right now if you add another story with the same name, the first one is
lost. For example: