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

feat: only shared plugins #17289

Merged
merged 1 commit into from
May 23, 2024
Merged

Conversation

patak-dev
Copy link
Member

We kept discussing about Environment Plugins today. Especially from the perspective of a plugin author and we are finding the fact that we have two different plugin types hard to work with:

  1. A plugin author may create a UnoCssEnvironmentPlugin that is a (environment) => EnvironmentPlugin to implement its internals. It can be used inside environmentPlugins but also exported. So the plugin will export both UnoCssPlugin and UnoCssEnvironmentPlugin. The user normally will use the shared plugin, and if it needs to have it per environment, it could use the UnoCssEnvironmentPlugin

Using the environment plugin sounds complex from the user side. It may not even be possible to do if there is some global configuration required without extra coordination. All plugins would also need to be reworked to expose both plugin types. We could think of environmentPlugins as an internal tool only, and not something we expose to users, but we can justify to add it in that case.

  1. One of the benefits of Environment Plugins was having access to the Environment instance. So there is no need for this.environment. Local state can also be used without issues. There is no need for a WeakMap<Environment, Data> to avoid cross-environment pollution.

This is great, but at the same time, it pushes us to have two mental models when authoring plugins. Environment Plugins can't be added to the main pipeline, and Shared Plugins can't be safely added to the environment pipeline (because some hooks may not be executed).

So, we're rethinking if we need two kinds of plugins at all. It may be better to enforce the use of this.environment for all plugins so they can be used in multiple environments or in a single one without any issues.

Instead of the environmentPlugins hook or the (environment) => EnvironmentPlugin ideas, we could go back to environment filtering. This was discussed when we started working on the Environment API proposal, but at that point it seemed that it didn't fit well. apply is run before config so that hook can't be used.
The idea would be to keep only shared plugins, and add a new applyForEnvironment(environment) hook (name to be discussed)

const UnoCssPlugin = () => {
  // shared global state
  return {
    buildStart() {
      // init per environment state with WeakMap<Environment,Data>, this.environmnet
      // we could also have this.data (the context is per plugin + environment already)
      // if we find a good way to type it
    },
    configureServer() {
      // use global hooks normally
    },
    applyForEnvironment(environment) {
      // return true if this plugin should be active in this environment
    },
    resolveId(id, importer) {
      // only called for enabled environments
    }
  }
}

We then have a single plugin type, and a way to define per environment plugins that don't diverge from current usage

Copy link

stackblitz bot commented May 23, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev patak-dev merged commit e36f2f3 into v6/environment-api May 23, 2024
12 of 13 checks passed
@patak-dev patak-dev deleted the feat/only-shared-plugins branch May 23, 2024 14:21
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.

1 participant