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

WIP: implement JSON story file format as indexer #22296

Closed

Conversation

floodfx
Copy link
Contributor

@floodfx floodfx commented Apr 27, 2023

Closes #20627

What I did

  • added loadCsfFromJson function in CsfFile.ts
  • added an indexer for .json files in preset.ts using above
  • removed warning for .json files in codegen-importfn-script.ts

How to test

Haven't added a test yet. Would like some feedback on above approach / code and answers to some of the TODOs particularly in loadCsfFromJson.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@chakAs3
Copy link
Contributor

chakAs3 commented Apr 28, 2023

How to test

Haven't added a test yet. Would like some feedback on above approach / code and answers to some of the TODOs particularly in loadCsfFromJson.

How to test is not about the unit-tests, it is how we can see your code working, like
1 - run yarn start,
2 - open specific story or create file.json then do ..

how did you see the result of your code ?

@floodfx
Copy link
Contributor Author

floodfx commented Apr 28, 2023

Yeah fair point. I have a local story in json format which I put into a test. It passes obviously but not sure what the snapshot should actually be. For example, where do the parameters go? Where should the story args end up?

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.

Thanks for doing this @floodfx !! I'm on vacation for the next few days but will take a closer look next week & do my best to give guidance.

@chakAs3
Copy link
Contributor

chakAs3 commented Apr 29, 2023

Hi @floodfx. I really like your intent, cause I have the same idea as basic with more capabilities like import-export stories, and generate default stories from a specific component library, it will be great, I can pair with you on this if you would like

@floodfx
Copy link
Contributor Author

floodfx commented Apr 29, 2023

@chakAs3 that would be great. As I mentioned in #20627, we use json stories in storybook 6 to test our server rendered templates for a golang project and it has worked well. I'd like to get that working with SB 7.

@chakAs3
Copy link
Contributor

chakAs3 commented Apr 29, 2023

@chakAs3 that would be great. As I mentioned in #20627, we use json stories in storybook 6 to test our server rendered templates for a golang project and it has worked well. I'd like to get that working with SB 7.

Yes sure @floodfx can you just add some test stories so I can start from there with you?

@chakAs3 chakAs3 changed the title code: initial attempt at json story indexer WIP: implement JSON story format as indexer Apr 29, 2023
@chakAs3 chakAs3 self-assigned this Apr 29, 2023
@chakAs3 chakAs3 changed the title WIP: implement JSON story format as indexer WIP: implement JSON story file format as indexer Apr 29, 2023
@chakAs3 chakAs3 marked this pull request as draft April 29, 2023 13:52
@floodfx
Copy link
Contributor Author

floodfx commented Apr 29, 2023

I added dc5a9b8 which is generally the format we use. I think the problem I have is I don’t know the extent of what the json story format should support and where these json story attributes should end up in the output?

@shilman
Copy link
Member

shilman commented May 8, 2023

@floodfx @chakAs3 thanks so much for putting this together and for your patience on this. The code looks good! I'm going to give a shot at restructuring it today to get this ready to merge.

@shilman shilman self-assigned this May 8, 2023
@shilman shilman mentioned this pull request May 8, 2023
3 tasks
@floodfx
Copy link
Contributor Author

floodfx commented May 8, 2023

thanks @shilman. lmk if I can be helpful.

@shilman
Copy link
Member

shilman commented May 9, 2023

Hi @floodfx, I merged #22460 which restructures this a little bit, and will release on the next 7.0.x patch release. Thanks so much for making this happen! 🙏

@shilman shilman closed this May 9, 2023
@floodfx
Copy link
Contributor Author

floodfx commented May 9, 2023

ok great! I am excited to try it out. thanks @shilman

@jonniebigodes
Copy link
Contributor

@floodfx, thank you so much for taking the time and effort into this pull request. We appreciate it 🙏 ! I want to follow up on one thing that was left out of this pull request. If you're ok with it, can you message me on our Discord Server (same username) so that we can go over it?

Looking forward to hearing from you.

Have a great day.

Stay safe.

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.

[Bug]: Storybook server needs an indexer for .stories.json files
4 participants