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

CLI: add type annotations in javascript main config files #20887

Merged
merged 2 commits into from
Feb 3, 2023

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Feb 2, 2023

Issue: N/A

What I did

I realized we could get type annotations in javascript for main.js too (thanks @JReinhold)

so this PR changes the generated main.js files to:

/** @type { import('@storybook/react-vite').StorybookConfig } */
export default {
  "stories": [
    "../src/**/*.mdx",
    "../src/**/*.stories.@(js|jsx|ts|tsx)"
  ],
};

It also adds tests to the file, which had none so far

How to test

  1. build the cli
  2. go to a js project without Storybook
  3. <path/to/storybook>/code/lib/cli/index.js init
  4. Assess the resulting main.js

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"]

@yannbf
Copy link
Member Author

yannbf commented Feb 2, 2023

Open question:

Given that in Typescript we generate this:

import { StorybookConfig from } '@storybook/react-vite'
const config: StorybookConfig {
  "stories": [
    "../src/**/*.mdx",
    "../src/**/*.stories.@(js|jsx|ts|tsx)"
  ],
};
export default config

We could potentially make js look like this and keep the files more consistent:

/** @type { import('@storybook/react-vite').StorybookConfig } */
const config = {
  "stories": [
    "../src/**/*.mdx",
    "../src/**/*.stories.@(js|jsx|ts|tsx)"
  ],
};
export default config

and probably in the docs we follow a similar pattern than for Typescript:

// Replace your-framework with the framework you are using (e.g., react-webpack5, vue3-webpack5)
/** @type { import('@storybook/your-framework).StorybookConfig } */
const config = {
  "stories": [
    "../src/**/*.mdx",
    "../src/**/*.stories.@(js|jsx|ts|tsx)"
  ],
};
export default config

WDYT @kasperpeulen @jonniebigodes @shilman ?

@shilman
Copy link
Member

shilman commented Feb 2, 2023

@yannbf That's really cool!! ❤️

To clarify: you're not saying get rid of the generated TS in CLI or documentation, but simply how to improve the JS side of things, right?

@yannbf
Copy link
Member Author

yannbf commented Feb 2, 2023

@yannbf That's really cool!! ❤️

To clarify: you're not saying get rid of the generated TS in CLI or documentation, but simply how to improve the JS side of things, right?

Yup, for sure!

@jonniebigodes
Copy link
Contributor

This is amazing @yannbf 🤩 ! If I understand this correctly, I think we can probably shrink down the number of code snippet files we have in our documentation. I'll follow up with you on it, and we'll go from there. Thank you both for thinking about this (@yannbf and @JReinhold ). Appreciate it 🙏

@kasperpeulen
Copy link
Contributor

I think this is great!

@jonniebigodes I don't think it would not reduce the code snippets, but make them more consistent right @yannbf ?

On the other hand, we should really just generate the JS examples from the TS examples with a codemod! @shilman What do you think, should be achievable right?

@shilman
Copy link
Member

shilman commented Feb 3, 2023

Interesting idea @kasperpeulen -- I like it as a post 7.0 task

@yannbf yannbf force-pushed the feat/type-annotations-in-js-config branch from 081aa68 to 7a994fa Compare February 3, 2023 16:23
@yannbf yannbf merged commit 904dba3 into next Feb 3, 2023
@yannbf yannbf deleted the feat/type-annotations-in-js-config branch February 3, 2023 18:10
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.

4 participants