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: back to environmentPlugins hook #16732

Merged

Conversation

patak-dev
Copy link
Member

Description

We're discussing about the API for per-environment plugins. We're seeing that using the functional version is making things quite hard for back compat and typing. This is an example returning a shared plugin and a per environment one

function Plugin() {
  // shared state
  return [
    {
      name: 'sp',
      configureServer(server) { ... },
    },
    (environment) => {
      // per env state
      return {
        name: 'pep',
        load(id) { ... },
      }
    }
  ]
}

The function looks quite nice, especially for inline plugins, but it has issues:

  • If we want to add a name, we need to use Object.defineProperty() (we would need this if we want to use per environment plugins in our internal plugins like vite:reporter, if we want to support filtering)
  • related to this, config.plugins will be a mix of Plugin | IsolatedPluginConstructor, and it breaks several ecosystem usage.
  • apply in the function and apply in the object with same name but diff meaning
  • I tried to implement enforce in the returned plugins, respecting the original position but is is really hard to do and full with edge cases. So we actually need enforce at the function plugins level.

We are thinking to get back to using a new hook to inject per environment plugins instead

function Plugin() {
  // shared state
  return {
    name: 'sp',
    configureServer(server) { ... },
    environmentPlugins(environment) {
      // per env state
      return {
        name: 'pep',
        load(id) { ... },
      }
    }
  }
}

This should still be quite nice for authoring, and it is explicit given the new hook name. A shared plugin can both have configureServer and return per environment plugins without the need of an array. All other plugin fields work as expected (enforce, apply, name, etc).
It was still good to try both options. The object form plus new hook is easier to work with for back compat. There may be some cases where ignoring the environmentPlugins hook could cause issues, but in most cases of config.plugins modification, it will work as is.

Copy link

stackblitz bot commented May 20, 2024

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

@patak-dev patak-dev merged commit 54c219e into v6/environment-api May 20, 2024
6 of 7 checks passed
@patak-dev patak-dev deleted the v6/environment-api-environment-plugins-hook branch May 20, 2024 20:34
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