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

Allow users to apply plugins to and customise the pre-bundling process #3124

Closed
nihalgonsalves opened this issue Apr 24, 2021 · 11 comments
Closed
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority)

Comments

@nihalgonsalves
Copy link
Member

nihalgonsalves commented Apr 24, 2021

This is to provide a common issue for various discussions.

Motivation / Use cases

  1. Plugin ecosystem Enable developers to create and use plugins that apply before prebundling

  2. Dependency patching Many dependencies have compatibility issues with esbuild. These historically have worked with other bundlers due to them being less strict or due to non-standard behaviour. These can often be unmaintained, blocking users from switching to Vite until they migrate away from such deps.

    • A workaround: patch-package – but this unfortunately does not work with pnpm
    • Adding dependencies to optimizeDeps.exclude – but since you're using this with dependencies that have compatibility issues, it often does not work since other issues require bundling or further fixes.
    • optimizeDeps.exclude + running esbuild separately for certain deps - here's a proof of concept for a customOptimizeDeps plugin, which is my current workaround for this use case.
  3. Non-standard setups It would be easier to migrate to Vite if you could work around issues such as fix: allow scan to use user provided loaders #1884 or Collisions with case sensitivity in imports #2861 using custom options/a plugin. These may not be recommended setups, but reduce friction when proposing a switch to a new build tool. Vite would not have to support such use cases natively, but would provide a useful escape hatch for those who need it.

Suggested solution

  • Provide a plugin hook that runs before pre-bundling, but doesn't require the user to opt-out of optimising dependencies. This would be especially valuable for plugin authors who want to hook into transforming dependencies before esbuild has touched them. This should cover many use cases, including patching dependencies. The advantage is that these plugins can be customised to apply to build, serve, or both, unlike esbuild plugins which would run only on serve (as the transform API doesn't support plugins). They also would stay compatible if esbuild is replaced with another pre-bundling setup, a concern that @patak-js expressed during a conversation in Vite Land.

  • Even with pre-pre-bundling plugins, we should nevertheless allow a custom esbuild config, similar to how we allow a custom rollup config if users need to further customise the build. This is something I outlined in feat: add optimizeDeps.esbuildOptions #2991, but it could be adapted a bit. For example, we could expose it as a custom hook instead, something like unstable_resolveEsbuildConfig, which would make it clear that it is an escape hatch and not stable.

Alternative

An entirely different approach for the esbuild config options could be to extract the current prebundling plugin fully, and allow users to replace it with their own plugin. However, this would likely be unnecessarily complex.

Additional context

N/A

cc @qnp

@nihalgonsalves nihalgonsalves added enhancement: pending triage enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority) and removed enhancement: pending triage labels Apr 24, 2021
@patak-dev
Copy link
Member

patak-dev commented Apr 24, 2021

Thanks for the write-up @nihalgonsalves. I think we'll need Evan's input for this one, but it would be good to discuss this directly with the PRs on the table.

About the config option, as we are already allowing the configuration of esbuild for the transform stage, I think we should also provide a config for the prebundling stage. And as a note, we are also exposing other config dependencies that may change in the feature like chokidar.

The hook sounds interesting, would you formalize a bit more what is the signature that you imagine and how you would use it for common cases?

@nihalgonsalves
Copy link
Member Author

nihalgonsalves commented Apr 24, 2021

Sure, a hook would be something added into DepOptimizationOptions with this signature (the naming is open of course):

// ...

import type { BuildOptions as EsbuildBuildOptions } from 'esbuild'

// ...

export interface DepOptimizationOptions {
  // ...

  /**
   * Hook called to resolve the esbuild config for dep scan and optimization
   */
  resolveEsbuildConfig: (buildConfig: EsbuildBuildOptions) => EsbuildBuildOptions
}

I find hooks more flexible than an object because they sidestep any issues with config merging strategies.

@patak-dev
Copy link
Member

Interesting, I think it may be a departure from the way other options already work. I think it would still make sense to have the object config so it is uniform with the rest of the config API. And if we do that, having two ways to do it in the config may be too much API surface.

Provide a plugin hook that runs before pre-bundling, but doesn't require the user to opt-out of optimising dependencies. This would be especially valuable for plugin authors who want to hook into transforming dependencies before esbuild has touched them. This should cover many use cases, including patching dependencies. The advantage is that these plugins can be customised to apply to build, serve, or both, unlike esbuild plugins which would run only on serve (as the transform API doesn't support plugins). They also would stay compatible if esbuild is replaced with another pre-bundling setup, a concern that @patak-js expressed during a conversation in Vite Land.

What would be the signature for this hook? And the cases that could be solved with it that are not possible using a possible depOptimization.esbuild option

@nihalgonsalves
Copy link
Member Author

nihalgonsalves commented Apr 24, 2021

having two ways to do it in the config may be too much API surface.

Yeah, that's a good argument to keep the object instead. It shouldn't be too much of a problem

What would be the signature for this hook?

I'm thinking of something like { enforce: 'pre-deps' } or similar.

And the cases that could be solved with it that are not possible using a possible depOptimization.esbuild option

The esbuild option would be able to solve most things. But I believe that a plugin should be the recommended way for plugins you want applied in both dev-server and build modes, since esbuild plugins would not be applied in build mode.

@qnp
Copy link
Contributor

qnp commented Apr 26, 2021

Hi,

According to @patak-js, I think we should keep the object config so ”it is uniform with the rest of the config API“ and it provides a simple way to configure Vite underlying processes.

To my point of view, as dev-server and build modes do not depend on the same technology (esbuild vs rollup), I think we should provide both ways to configure either one or the other explicitly – as problems and edge cases we would like to cover will be specifically for one given tool and not the other one.

If I understand you @nihalgonsalves quite well, the point of using the plugin solution is to provide a way for plugin authors to cover these cases without using the vite config ?

@nihalgonsalves
Copy link
Member Author

@qnp

To my point of view, as dev-server and build modes do not depend on the same technology (esbuild vs rollup), I think we should provide both ways to configure either one or the other explicitly – as problems and edge cases we would like to cover will be specifically for one given tool and not the other on

I agree - that's why I proposed both additions. If something is not specific to esbuild/rollup, it would be recommended to use a generic plugin that applies before the optimisation process, so that it works for both modes. But if a user needs to customise the esbuild/rollup config for something, that should also be allowed.

If I understand you @nihalgonsalves quite well, the point of using the plugin solution is to provide a way for plugin authors to cover these cases without using the vite config ?

Yes, to be able to extend functionality without needing to add niche use cases/legacy support/etc to Vite, or fork/patch Vite.

@patak-dev
Copy link
Member

We already can detect if you are in serve or build mode in plugins and could resolve some dependencies (using resolveId, load, transform hooks) in a different way, no? Could you create an example of how this new plugin hook enables something that is not possible or really contrived to do (once we get the optimizeDeps.esbuild config)?

@nihalgonsalves
Copy link
Member Author

@patak-js Here's an example that I currently need to work around:

There's a dep that has an invalid import: bvaughn/react-virtualized#1635 (bvaughn/react-virtualized#1632, bvaughn/react-virtualized#1212)

I have an esbuild plugin that removes that line from the code:

build.onLoad(
  {
    filter: /react-virtualized\/dist\/es\/WindowScroller\/utils\/onScroll\.js$/,
  },
  async args => {
    const jsContents = await fs.promises.readFile(args.path, 'utf8')
    return {
      contents: jsContents.replace(
        'import { bpfrpt_proptype_WindowScroller } from "../WindowScroller.js";',
        '',
      ),
    }
  },
)

I'd have to write an additional rollup plugin for something likes this. Luckily rollup doesn't fail on this error and simply prints a warning, but you'd otherwise have to also write a build plugin.

The same applies to any transformation a plugin would want to apply before esbuild sees the code. You'd likely want the same transformation in production, so you have to write two plugins. A common pre-dep-optimisation plugin would fix the problem at the source.

However, adding the esbuild options would already unblock many use cases, and we should probably go ahead with that first.

@huyansheng3
Copy link

@nihalgonsalves does it work for vitejs ?

@bluwy
Copy link
Member

bluwy commented Mar 6, 2022

@nihalgonsalves Are there more to work on for this issue? vite-plugin-svelte has been extensively using pre-bundling for Svelte files. I've made some changes like #6801 to make it possible to prebundle libraries with raw Svelte components. I guess the caveat now is that an esbuild plugin is need to get into the pre-pre-bundling phase, but having Vite plugins to participate in the prebundling process has been talked about at #4921, and would require a fair amount of refactoring to do it right.

@bluwy
Copy link
Member

bluwy commented Jun 13, 2022

I think this is resolved with optimizeDeps.esbuildOptions, optimizeDeps.esbuildOptions.plugins and optimizeDeps.extensions. The other points seems to relate to #4921. I'll go ahead and close this then.

@bluwy bluwy closed this as completed Jun 13, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

No branches or pull requests

5 participants