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

Core: Throw an error if renderer is used as framework #19452

Merged
merged 5 commits into from
Oct 18, 2022

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Oct 12, 2022

Issue:

When upgrading from 6.5 to 7.0, if you just upgrade the versions in package.json instead of running the CLI, and you don't change the framework setting, you'll get an error like this:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './preset' is not defined by "exports" in /Users/me/my-project/node_modules/@storybook/react/package.json

It's not clear from this error what the problem is, and it looks like storybook is just broken.

What I did

This PR checks the framework against the list of 6.5 renderers that are not frameworks in 7.0 (angular and ember are frameworks). If one of the "renderers" are found, we will now throw an error that gives a hint of what is actually wrong and how to fix it:

ERR! Error: Invalid value of @storybook/react in the 'framework' field of Storybook config.
ERR! Please see the v7 Migration guide for more information:
ERR! https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#framework-field-mandatory

I went back-and-forth with myself whether I should use a permalink, but I think it's best to have up-to-date docs here, especially since the list of valid frameworks will expand.

Also, I was going to add a test for this, and I found the dev-server.test.ts file which was great, but it turned out to be for a utility function instead, so I moved it. I can remove that commit though if you'd like, since it's not directly related to this PR.

How to test

The way I tested was to build the branch, create a sandbox (linked), and set frameworks to @storybook/react. Running yarn storybook then showed my validation error, instead of the confusing one.

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.

Nice one @IanVS. WDYT about prompting users to upgrade with sb upgrade or to run sb automigrate since those can fix a lot of problems, not just this particular one?

@IanVS IanVS requested a review from shilman October 17, 2022 13:01
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.

LGTM! 💯

@shilman shilman self-assigned this Oct 17, 2022
@shilman shilman merged commit dd96c9e into next Oct 18, 2022
@shilman shilman deleted the cli/old-framework-error branch October 18, 2022 14:28
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.

2 participants