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

Vite: Fix framework option checks, and SSv6 #19062

Merged
merged 4 commits into from
Sep 1, 2022
Merged

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Aug 31, 2022

Issue:

We used to be able to get a simple react or svelte from options.framework, but no longer. This adds a bit of a helper to check whether the framework is a string or an object, and return the value. It also adjusts the format of the expected returns to be @storybook/react-vite for instance. This should be merged before #19026, which will need to be updated a bit after these changes.

What I did

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@IanVS IanVS changed the title Fix framework option checks Vite: Fix framework option checks, and SSv6 Aug 31, 2022
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.

I think this is duplicative to work that's already been done on the webpack side of the house.

// always returns an object
const frameworkOptions = presets.apply('frameworkOptions');

I'd propose we move this utility into the common preset and we try to move all framework-specific code out of the vite builder. I'm fine with leaving it in now since we're in transition.

Also reactOptions / svelteOptions will be removed in 7.0 & there will be a migration:

export const framework = {
  name: '@storybook/react-vite';
  options: {
    // former reactOptions
  }
}

@IanVS
Copy link
Member Author

IanVS commented Aug 31, 2022

Thanks @shilman. I pulled the utility up to core-common and used it in webpack as well. And I fixed the frameworkOptions like you pointed out. Appreciate it!

@IanVS IanVS requested a review from shilman August 31, 2022 12:25
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 merged commit fb4b783 into next Sep 1, 2022
@shilman shilman deleted the vite-framework-checks branch September 1, 2022 02:02
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