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: Add vue-vite framework for Vue2 #19230

Merged
merged 17 commits into from
Oct 3, 2022
Merged

Vite: Add vue-vite framework for Vue2 #19230

merged 17 commits into from
Oct 3, 2022

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Sep 22, 2022

Issue: #18293

What I did

This adds a new framework for vue2 and vite, @storybook/vue-vite. It also adds a reproduction, but it sounds like we may need to comment that out for now until we get vue2 playing nicely in CI.

TODO:

How to test

  • Run yarn bootstrap --build
  • Run yarn generate-repros-next --template vue2-vite/2.7-js --local-registry
  • cd ../repros/vue2-vite/2.7-js/after-storybook
  • yarn storybook

Otherwise we end up with type problems between different versions.
It's only used in the react framework, and doesn't need to be exported
Webpack doesn't need them, but Vite does
@IanVS
Copy link
Member Author

IanVS commented Sep 22, 2022

I didn't deploy my repro template, which sounds like it might break CI due to the vue2 renderer not working. So, I'd say this PR isn't ready to merge, and is blocked by #19207.

'vue2-vite/2.7-js': {
name: 'Vue2 Vite (vue 2.7 JS)',
script:
'yarn create vite . --template vanilla && yarn add --dev @vitejs/plugin-vue2 vue-template-compiler vue@2 && echo "import vue2 from \'@vitejs/plugin-vue2\';\n\nexport default {\n\tplugins: [vue2()]\n};" > vite.config.js',
Copy link
Member

Choose a reason for hiding this comment

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

@ndelangen WDYT about this? we said we were going to only use community bootstrap tools for this. do we use this as a placeholder for now & then move to some storybook package later?

Copy link
Member

Choose a reason for hiding this comment

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

Let's discuss this, I'm obviously not a fan of this, because in 2 weeks no-one will know why any of it is here, and it won't reproduce anything users are actually using.

Copy link
Member Author

Choose a reason for hiding this comment

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

The challenge is, there's no generator / bootstrap for using Vue 2 with Vite, as far as I could find. Vue 2 had its own cli, and it's necessary to create a Vue 2 project and then manually convert it over to Vite. Or, this approach which required fewer changes, which is to add a vanilla Vite project, and then add Vue2 into it.

Copy link
Member

Choose a reason for hiding this comment

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

understood! thank you for the explanation, let's do a follow up at some point to extract this into the public domain in some sort of CLI project generator.

@IanVS
Copy link
Member Author

IanVS commented Sep 26, 2022

I'm going to leave a TODO comment in the code to say that we should create a npm create script for vue2 + vite, and we'll move over to that as soon as it's published.

Also, I'll make a PR to remove the extraPackages from the vue/svelte generators to remove the loaders, and see what happens.

@IanVS IanVS requested a review from ndelangen September 27, 2022 00:44
@ndelangen
Copy link
Member

@ndelangen ndelangen assigned IanVS and unassigned ndelangen Sep 27, 2022
@IanVS
Copy link
Member Author

IanVS commented Sep 27, 2022

I think the issue is that I did not actually build and release the repro into https://github.com/storybookjs/repro-templates-temp. I was reluctant to do that until #19204 is fixed, since otherwise I guess CI will start to fail everywhere, right @shilman? I guess this is on-hold until that's figured out. Sorry, shouldn't have tagged you for review Norbert, I forgot that was still a problem.

@naquiroz
Copy link

Waiting for this to be merged here

@shilman shilman changed the title Vite: Add vue-vite framework (Vue2) Vite: Add vue-vite framework for Vue2 Oct 3, 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.

@IanVS @tmeasday

There are some messed up chromatic snapshots (controls, etc.), also failing in my previous Vue2 PR. I'm going to merge this PR and follow up separately. It's too painful to deal with the lack of cached repro template. In the future, we should split up the PRs so that we can merge the repro templates first and then iterates on CI errors with a stable repro.

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