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

CHANGE to use getDataForManager over extract api on storyStore #11584

Merged
merged 7 commits into from
Jul 21, 2020

Conversation

ndelangen
Copy link
Member

Issue: #11302

What I did

I improved the sb extract cli command to use a better StoryStore api.

lib/cli/src/extract.ts Outdated Show resolved Hide resolved
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe you want to do the earlier parameter filtering to globalParameter, each of the kindParameters and all the stories' individual parameters.

@tmeasday
Copy link
Member

Although maybe we should add that code to the store itself so you call call window.__STORYBOOK_STORY_STORE__.getStoriesJsonData or the like?

@ndelangen
Copy link
Member Author

ndelangen commented Jul 17, 2020

maybe just window.__STORYBOOK_STORY_STORE__. getDataForJSON()?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a couple suggestions

lib/client-api/src/story_store.ts Outdated Show resolved Hide resolved
@@ -569,6 +570,35 @@ export default class StoryStore {
};
};

getStoriesJsonData = () => {
const value = this.getDataForManager();
const allowed = ['fileName', 'docsOnly', 'framework', '__id'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__isArgsStory?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to know that for the sidebar?

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now! I agree mapValues is better. Also maybe we can type it?

@ndelangen
Copy link
Member Author

Cool I added a test as well

@ndelangen ndelangen requested a review from shilman July 21, 2020 08:53
@ndelangen ndelangen merged commit 7ef6f0a into next Jul 21, 2020
@ndelangen ndelangen deleted the tech/change-sbextract-cli-command-to-use-better-api branch July 21, 2020 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants