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

[Bug]: migrate csf-2-to-3 does not work for CSF1 #22408

Open
Demi1024 opened this issue May 5, 2023 · 6 comments
Open

[Bug]: migrate csf-2-to-3 does not work for CSF1 #22408

Demi1024 opened this issue May 5, 2023 · 6 comments

Comments

@Demi1024
Copy link

Demi1024 commented May 5, 2023

Describe the bug

After executing the command, the blank line of the file is deleted without any migration modification

 npx storybook@latest migrate csf-2-to-3 --glob="src/**/*.stories.js"

pic for result
image
image

To Reproduce

No response

System

Environment Info:

  System:
    OS: macOS 12.3.1
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 16.13.1 - /usr/local/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.1.2 - /usr/local/bin/npm
  Browsers:
    Chrome: 112.0.5615.137
    Firefox: 110.0.1
    Safari: 15.4
  npmPackages:
    @storybook/addon-actions: ^7.0.7 => 7.0.8
    @storybook/addon-console: ^1.2.3 => 1.2.3
    @storybook/addon-links: ^7.0.7 => 7.0.8
    @storybook/addons: ^7.0.7 => 7.0.8
    @storybook/cli: ^7.0.7 => 7.0.8
    @storybook/client-api: ^7.0.7 => 7.0.8
    @storybook/react: ^7.0.7 => 7.0.8
    @storybook/react-webpack5: ^7.0.7 => 7.0.8
    @storybook/theming: ^7.0.7 => 7.0.8

Additional context

No response

@shilman
Copy link
Member

shilman commented May 8, 2023

Do you a have a reproduction repo you can share? If not, can you create one? Go to https://storybook.new or see repro docs. Thank you! 🙏

@Demi1024
Copy link
Author

Demi1024 commented May 9, 2023

yes, I write a story for header by csf2. link
When I execute the migration command, it doesn't take effect
image

I'm working on a storybook 6.x to 7.x upgrade these days. I've found that if the property is configured using csf2, the story will automatically upgrade, but just exporting a component won't work.

@lucapollani
Copy link

I've encountered the same behavior, no changes performed.
The only file changed had spacing changes.

image

@shilman shilman added the sev:S2 label Jan 12, 2024
@shilman shilman added this to the 8.0-beta milestone Jan 12, 2024
@vanessayuenn vanessayuenn moved this from Waiting to Empathy Backlog in Core Team Projects Jan 12, 2024
@vanessayuenn vanessayuenn moved this from Empathy Backlog to Empathy - Ready for work in Core Team Projects Jan 23, 2024
@vanessayuenn
Copy link
Contributor

vanessayuenn commented Jan 23, 2024

Dupe to #25226, which was resolved in #25708.

not quite a dupe, as elaborated below 👇🏼

@github-project-automation github-project-automation bot moved this from Empathy - Ready for work to Done in Core Team Projects Jan 23, 2024
@vanessayuenn vanessayuenn reopened this Jan 23, 2024
@vanessayuenn vanessayuenn moved this from Done to Empathy Backlog in Core Team Projects Jan 23, 2024
@vanessayuenn vanessayuenn moved this from Empathy Backlog to Empathy - Ready for work in Core Team Projects Jan 23, 2024
@shilman
Copy link
Member

shilman commented Jan 23, 2024

The issue here is that the linked example is CSF1 not CSF2. The difference between CSF1 and CSF2 is that the former specifies story functions with no args and the latter accepts args and renders them in your component or in a composition of components.

We still support CSF 1, 2, and 3, and I believe that for "no-arg" stories, CSF1 is still the best format. I wonder if this is a documentation issue, or whether the migration should print something out in this case to let the user know that skipping this story is intentional, e.g. Skipping path/to/Component:StoryName since it does not accept args .... Skipped N stories that do not accept args [link to docs or inline explanation]

@vanessayuenn vanessayuenn changed the title [Bug]: migrate csf-2-to-3 not work [Bug]: migrate csf-2-to-3 does not work for CSF1 Jan 23, 2024
@vanessayuenn
Copy link
Contributor

the migration should print something out in this case to let the user know that skipping this story is intentional, e.g. Skipping path/to/Component:StoryName since it does not accept args .... Skipped N stories that do not accept args [link to docs or inline explanation]

note to whoever is picking this up: this ☝🏼 should be the task for resolving this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: No status
Development

No branches or pull requests

5 participants