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: Improve support of mono repositories #23458

Merged
merged 19 commits into from
Jul 21, 2023
Merged

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Jul 14, 2023

Closes #21216
Closes #21710

What I did

Improved support for mono repositories to apply the "wrapForPnp" wrapper also in mono repository environments. Additionally, an auto migration was added.

Improved support for projects which are part of a workspace configured by yarn, npm, pnpm, lerna, turbo or rush

How to test

  • Go into a project which is part of a mono repository setup or where yarn pnp is enabled.
  • If the project is a yarn workspace with pnp enabled, please run yarn exec <storybook-project-folder>/code/lib/cli/bin/index.js automigrate for the automigration or yarn exec <storybook-project-folder>/code/lib/cli/bin/index.js init to initialize Storybook. Otherwise, you can ditch the yarn exec part.
  • Please check whether the values for the configuration fields in main.ts are properly wrapped by the wrapForPnp or getAbsolutePath wrapper.
  • Play around with the following origin scenarios:
    • a wrapForPnp function is already in place, but is not used to e.g. wrap addon values
    • test it for main.ts and main.js files.

Tested scenarios:

  • init
  • auto-upgrade
    • where no fields were transformed
    • where all fields were already transformed (wrap-require automigration skipped)
    • where some fields were transformed (do not introduce a new wrapper if getAbsolutePath or wrapForPnp functions are in place)
    • checked for Typescript and Javascript

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

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
  • Make sure this PR contains one of the labels below.

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

@valentinpalkovic valentinpalkovic added bug patch:yes Bugfix & documentation PR that need to be picked to main branch ci:daily Run the CI jobs that normally run in the daily job. and removed ci: do not merge labels Jul 18, 2023
@valentinpalkovic valentinpalkovic changed the title Improve support of monorepositories Improve support of mono repositories Jul 18, 2023
@valentinpalkovic valentinpalkovic changed the title Improve support of mono repositories CLI: Improve support of mono repositories Jul 18, 2023
@valentinpalkovic valentinpalkovic marked this pull request as ready for review July 19, 2023 09:03
@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-monorepository-setups branch from cda3ae7 to e2a33c5 Compare July 19, 2023 09:16
Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Changes all look great! I haven't QA'd this but I plan to do it later this week! I'm curious what will happen in scenarios where people have a "base" main.js which is used in other main.js files e.g.

import baseConfig from '../../../storybook-common/main'

export default {
  ...baseConfig,
  stories: ['...']
}

@valentinpalkovic valentinpalkovic self-assigned this Jul 19, 2023
@valentinpalkovic
Copy link
Contributor Author

valentinpalkovic commented Jul 19, 2023

Changes all look great! I haven't QA'd this but I plan to do it later this week! I'm curious what will happen in scenarios where people have a "base" main.js which is used in other main.js files e.g.

What should happen? -> Nothing will happen ;) We are doing AST analysis, so we don't follow the path of their base config. They would have to apply the changes manually. There is no chance for us to figure out whether we have to run the auto migrations. An edge case we can safely ignore. It's not worth supporting it in the auto migrations.

@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-monorepository-setups branch from c08bba7 to 4c3e892 Compare July 20, 2023 13:56
@yannbf yannbf merged commit 5b34161 into next Jul 21, 2023
@yannbf yannbf deleted the valentin/fix-monorepository-setups branch July 21, 2023 13:34
@github-actions github-actions bot mentioned this pull request Jul 21, 2023
14 tasks
JReinhold pushed a commit that referenced this pull request Jul 24, 2023
…y-setups

CLI: Improve support of mono repositories
(cherry picked from commit 5b34161)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jul 24, 2023
legnaleurc pushed a commit to legnaleurc/storybook that referenced this pull request Sep 6, 2023
In monorepo, the framework name will be an absolute path.

A follow up for storybookjs#23458.
legnaleurc pushed a commit to legnaleurc/storybook that referenced this pull request Sep 6, 2023
In monorepo, the framework name will be an absolute path.

A follow up for storybookjs#23458.
legnaleurc added a commit to legnaleurc/storybook that referenced this pull request Sep 7, 2023
In monorepo, the framework name will be an absolute path.

A follow up for storybookjs#23458.
legnaleurc added a commit to legnaleurc/storybook that referenced this pull request Sep 7, 2023
In monorepo, the framework name will be an absolute path.

A follow up for storybookjs#23458.
legnaleurc added a commit to legnaleurc/storybook that referenced this pull request Sep 8, 2023
In monorepo, the framework name will be an absolute path.

A follow up for storybookjs#23458.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug ci:daily Run the CI jobs that normally run in the daily job. patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Storybook 7 migration not sucessful in monorepo [Bug]: Failed to load preset: "@storybook/**/preset"
2 participants