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(storybook): add args to auto generate docs #5979

Merged
merged 3 commits into from
Jul 22, 2022

Conversation

virtuoushub
Copy link
Collaborator

Without this, we don't get the Doc Block / "Show code" feature Storybook provides without having the framework user modify their stories files.

image

see:

@netlify
Copy link

netlify bot commented Jul 16, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit ff515b1
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62da91e762278c0009b24d1e

@jtoar
Copy link
Contributor

jtoar commented Jul 16, 2022

@virtuoushub nice! You'll have to update the CLI's fixtures to get tests passing:

yarn workspace @redwoodjs/cli run test -u

@virtuoushub virtuoushub force-pushed the fix/storybook/enable-doc-block branch from 0a9ee2a to 1b91afc Compare July 18, 2022 13:42
virtuoushub added a commit to virtuoushub/redwood that referenced this pull request Jul 18, 2022
```sh
yarn workspace @redwoodjs/cli run test -u
```

see: redwoodjs#5979 (comment)
@virtuoushub virtuoushub force-pushed the fix/storybook/enable-doc-block branch from 3d9acc9 to af6bf60 Compare July 19, 2022 00:45
virtuoushub added a commit to virtuoushub/redwood that referenced this pull request Jul 19, 2022
```sh
yarn workspace @redwoodjs/cli run test -u
```

see: redwoodjs#5979 (comment)
@virtuoushub virtuoushub force-pushed the fix/storybook/enable-doc-block branch from af6bf60 to 86c95cd Compare July 21, 2022 21:38
@nx-cloud
Copy link

nx-cloud bot commented Jul 21, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ff515b1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 14 targets

Sent with 💌 from NxCloud.

Copy link
Contributor

@jtoar jtoar left a comment

Choose a reason for hiding this comment

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

Works!

Note that I almost bikeshed (bikeshed-ed?) and suggested we rename args to props, but storybook refers to it consistently as args, so let's follow thier lead.

@jtoar jtoar enabled auto-merge (squash) July 22, 2022 12:13
@jtoar jtoar disabled auto-merge July 22, 2022 12:14
@jtoar jtoar enabled auto-merge (squash) July 22, 2022 12:16
@jtoar jtoar merged commit cfb19a0 into redwoodjs:main Jul 22, 2022
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jul 22, 2022
dac09 added a commit to dac09/redwood that referenced this pull request Jul 22, 2022
…ctmode-gen

* 'main' of github.com:redwoodjs/redwood:
  fix(deps): update dependency eslint-plugin-jsx-a11y to v6.6.1 (redwoodjs#6017)
  chore(deps): update dependency @playwright/test to v1.24.0 (redwoodjs#6019)
  chore(deps): update dependency @okta/jwt-verifier to v2.6.0 (redwoodjs#6018)
  fix(storybook): add `args` to auto generate docs (redwoodjs#5979)
  fix: `yarn rw setup auth clerk` fails in canary (redwoodjs#5998)
  Provide a possibility to disable flows in dbAuth completely (redwoodjs#5851)
@jtoar jtoar removed this from the next-release milestone Jul 22, 2022
@jtoar jtoar added this to the v2.2.0 milestone Jul 22, 2022
@Philzen
Copy link
Contributor

Philzen commented Jul 24, 2022

Just accidentally browsing across these changes as i was made aware of this issue before. Just wondered: does this issue happen at all using CSFv3?

See https://storybook.js.org/blog/component-story-format-3-0/

With CSFv3, the whole story boils down to even less code:

export const Loading = {}

... that's it, that's the default story :)

Wondering whether this behaves correctly even if Error is not implemented in the cell. Otherwise, we'd need to add the check to the render function:

export const Loading = {
  render: (args) => Loading ? <Loading {...args} /> : null
}

so we wouldn't win a lot in the Cell story generators, but still a whole lot in all the others.

@virtuoushub virtuoushub deleted the fix/storybook/enable-doc-block branch July 25, 2022 20:00
@virtuoushub
Copy link
Collaborator Author

Just accidentally browsing across these changes as i was made aware of this issue before. Just wondered: does this issue happen at all using CSFv3?

See https://storybook.js.org/blog/component-story-format-3-0/

With CSFv3, the whole story boils down to even less code:

export const Loading = {}

... that's it, that's the default story :)

Wondering whether this behaves correctly even if Error is not implemented in the cell. Otherwise, we'd need to add the check to the render function:

export const Loading = {
  render: (args) => Loading ? <Loading {...args} /> : null
}

so we wouldn't win a lot in the Cell story generators, but still a whole lot in all the others.

Hi @Philzen - I started going down this path, and yes; I think I got slowed up on the Cells and never finished it. I agree we gain a lot by adopting CSF 3.0 in terms of boilerplate reduction. Will add it to #5269 so we track

jtoar pushed a commit to orgtoar/redwood-test that referenced this pull request Jul 28, 2022
* fix(storybook): add `args` to auto generate docs

see:
- storybookjs/storybook#8104 (comment)
- https://storybook.js.org/docs/react/writing-docs/doc-block-source

* test(unit): update snapshots

```sh
yarn workspace @redwoodjs/cli run test -u
```

see: redwoodjs/redwood#5979 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

None yet

4 participants