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

refactor: move framework plugins out of core #11158

Merged
merged 2 commits into from
Dec 3, 2022

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Dec 2, 2022

Description

Benefits of Framework plugins being out of Core

Foster ownership of the plugin from folks out of the Core team

The framework plugins in the core monorepo doesn't help to encourage collaborators to move from one-off contributions and bug fixes to take responsibility for the long-term future of the plugins (especially in the case of plugin-react since there are several Vue core team members to look for plugin-vue). Some problems:

  • Issues and PRs for the plugin are mixed with the currently ~500 issues and ~150 PRs from Core
  • Discussions are also mixed
  • Contributors to the plugin will be unable to show as top contributors
  • We can't give maintainer access easily to contributors only for the plugin

Independents repository under the @vitejs org would solve these issues

Reduce CI time, avoid building and fully-testing plugin-react for each PR/commit

With vite-ecosystem-ci available, we can include @vitejs/plugin-react CI and run it on demand when needed and before each release.

We also avoid downloading deps and re-building pluing-react in vite-ecosystem-ci when we aren't testing React (right now we need to do this when testing every other CI).

This becomes more important now that we have two React plugins (one based on esbuild+babel, and the other one on esbuild+SWC).

Double down Vite's framework agnostic story

When Vite's API was still in flux and vite-ecosystem-ci wasn't there, I think it was justified to have Plugin React and Plugin Vue in core. Right now, IMO it is the other way around. It is unfair for framework plugins out of core and pushes us to take shortcuts that we may not do if the repos were separated.

Untangle the plugin release cycle from Vite core

@vitejs/plugin-react and other plugins could have an independent release cycle. We would avoid releasing the plugin when it is using not yet released APIs from Vite core, something that is hard to test if the plugin is in core. We broke plugin-react a few times in the past year due to this.

Moving forward

We decided we should move plugin-react, plugin-vue, and plugin-vue-jsx as independent repositories under the vitejs org as part of Vite 4.

Other Plugins

I think plugin-legacy should remain in core. An unrelated proposal would be to move create-vite out of core too. We already discussed this with @bluwy, in the context of https://github.com/bluwy/create-vite-extra. The PRs to the templates and discussions around what framework to include would also be better off out of the core monorepo IMO. But we can discuss this for Vite 5.

Repositories

Core

Vue

React

Testing infra

For now, all the Vue and React tests are duplicated in core and in the new repositories. There are Vite features we are testing in some of these. Ideally we should extract some of these, we may not need to test for core as much of Vue and React as we do now.

The extracted plugins will be added to vite-ecosystem-ci to be tested as part of the ecosystem CI infra as we already do with all other frameworks.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@patak-dev patak-dev changed the title refactor: move famework plugins out of core refactor: move framework plugins out of core Dec 2, 2022
@patak-dev
Copy link
Member Author

The new repositories are now being tested as part of vite-ecosystem-ci:

@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Dec 2, 2022

📝 Ran ecosystem CI: Open

suite result
astro ❌ failure
histoire ❌ failure
iles ❌ failure
ladle ❌ failure
laravel ✅ success
marko ✅ success
nuxt-framework ❌ failure
rakkas ❌ failure
sveltekit ❌ failure
vite-plugin-ssr ❌ failure
vite-plugin-react ❌ failure
vite-plugin-svelte ✅ success
vite-plugin-vue ❌ failure
vite-setup-catalogue ❌ failure
vitepress ❌ failure
vitest ❌ failure
windicss ✅ success

@patak-dev
Copy link
Member Author

I'll PR tomorrow a fix for vite-ecosystem-ci

bluwy
bluwy previously approved these changes Dec 3, 2022
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

Changes looks good to me 👍

sapphi-red
sapphi-red previously approved these changes Dec 3, 2022
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

LGTM 🎈

@patak-dev patak-dev dismissed stale reviews from sapphi-red and bluwy via 607ff09 December 3, 2022 10:05
@patak-dev patak-dev requested a review from sapphi-red December 3, 2022 10:16
@patak-dev
Copy link
Member Author

We discussed with @domininkg and @sapphi-red, and decided that we should remove the now duplicated Vue Rnd react playgrounds from core directly in this PR because testing works better if they are run in their own repository.

We may want to port some tests from there to other playgrounds as we discover they are useful outside of the context of the framework.

I removed the ones that were duplicated from the PR, but there are some other playgrounds that use the plugins (and I haven't moved these):

  • optimize-deps (@vitejs/plugin-vue)
  • preload (@vitejs/plugin-vue)
  • tailwind (@vitejs/plugin-vue)
  • worker (@vitejs/plugin-vue-jsx)
  • file-delete-restore (@vitejs/plugin-react)

These aren't specifically testing the framework is using, it is just for historic reasons or because it was more comfortable to use a framework to implement the playground. With time, we should rewrite them, but we could still go forward with the PR with them as is.

CI is down from 8min to 6min after removing these.

For reference, the problem is that if there is an API change in Vite 5 that requires a release for these plugins, then we have to be doing a lot of paperwork just to keep the playgrounds functional (and even accept some failing PRs to accomplish it)
So we could rework these and split them into what is useful for the plugins (and move them out to the repos) and what is useful to core (and remove the dep with the plugins). But we can do this in the next months before we start Vite 5. This not only affects these plugins, I think it may be better to move the tailwind playground out for example. These should be in tailwinds CI, and we should add that CI to vite-ecosystem-ci.

@patak-dev patak-dev merged commit 5935619 into main Dec 3, 2022
@patak-dev patak-dev deleted the refactor/move-framework-plugins-out-of-core branch December 3, 2022 10:36
@fvsch fvsch mentioned this pull request Dec 5, 2022
7 tasks
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants