-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Comments
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? |
Sure, a hook would be something added into // ...
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. |
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.
What would be the signature for this hook? And the cases that could be solved with it that are not possible using a possible |
Yeah, that's a good argument to keep the object instead. It shouldn't be too much of a problem
I'm thinking of something like
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 |
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 ? |
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.
Yes, to be able to extend functionality without needing to add niche use cases/legacy support/etc to Vite, or fork/patch Vite. |
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)? |
@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. |
@nihalgonsalves does it work for vitejs ? |
@nihalgonsalves Are there more to work on for this issue? |
I think this is resolved with |
This is to provide a common issue for various discussions.
Motivation / Use cases
Plugin ecosystem Enable developers to create and use plugins that apply before prebundling
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.
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 acustomOptimizeDeps
plugin, which is my current workaround for this use case.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 ifesbuild
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
The text was updated successfully, but these errors were encountered: