esbuild icon indicating copy to clipboard operation
esbuild copied to clipboard

Call all plugins onResolve instead of using only the first one

Open remorses opened this issue 5 years ago • 3 comments

Currently only the first plugin onResolve result will be used, this means there is no way to override a previous plugin result

Calling all plugins allows you to override previous plugins

remorses avatar Nov 01 '20 16:11 remorses

I'd be worried about performance implications

One way to deal with that is by default plugins don't allow other plugins. In the object the plugin returns, the plugin could do something like:

// ... plugin code
     return { contents: code, runOtherPlugins: true}
// ... rest of plugin code

And then, if the plugin doesn't set runOtherPlugins: true or if you want to force it to run other plugins, on the config object for ESBuild, maybe you could still override that, something like:

build({
   // ...esbuild config

   plugins: [
    [plugin, {runOtherPlugins: true}]
   ],
})

This assumes you usually don't want plugins to run other plugins, which is probably the case for every file type except JavaScript/TypeScript

Jarred-Sumner avatar Nov 10 '20 03:11 Jarred-Sumner

@Jarred-Sumner i think there would be no performance degradation, a plugin can early return a null value to skip itself from running

remorses avatar Nov 10 '20 09:11 remorses

I met the exact same needs as you, @remorses . But your PR only modifies the TS part for esbuild. I believe there is another GO part: https://github.com/evanw/esbuild/blob/master/internal/bundler/bundler.go#L587-L640.

And I would argue that the api of onResolve and onLoad may should be changed to act more like middlewares, and to be more intuitive. @evanw Here is my draft proposal on a more middleware-like plugin system, hope it can be a good reference:

EDIT: Actually, I can write a middleware-like plugin system that accepts middleware-like plugin, as a esbuild plugin, if the proposal is not acceptable. I guess that is what the author originally thought (pluginName), a plugin for plugins.

  1. To repeatedly call plugins, basically it will be a loop like
for(;plugins.some(p => p.canApply());)
  p.apply();

So better no special treat for file systems, like what the comment in https://github.com/evanw/esbuild/issues/546#issuecomment-730121484. My point is to make entrypoint same as import paths. A simpler and consistent api would make the loop more easier to implement.

  1. OnResolve api may like:
// the first argument maybe a simple path filter REGEXP
// or a function that can judge if we should try to resolve
onResolve(string | (OnResolveArgs) => boolean, (onResolveArgs) => onResolveResult)

// has the previous resolve result, and the importer
interface OnResolveArgs extends onResolveResult {
  importer?: onResolveResult;
}

interface OnResolveResult {
  path?: string;
  external?: boolean;
  errors?: Message[];
  warnings?: Message[];
  namespace?: string; // or 'the message from the previous plugin'
  // maybe other type than string, if we want more power context sharing
}
  1. onLoad will be similar except for loader field. I would remove the loader field, and have some builtin namespaces like esbuild-js-loader as a hint to pass the control to esbuild loader, esbuild-stop to terminate the loop, or some other custom hints to continue transform.
// a namespace filter REGEXP string, or full custom filter function
onLoad(string | (OnLoadResult) => boolean, (OnLoadResult) => OnLoadResult)

interface OnLoadResult {
  contents?: string | Uint8Array;
  errors?: Message[];
  warnings?: Message[];
  namespace?: string;
}
  1. By the means of no special treatment for files, plugins can be applied to transform api for better integration.

xhebox avatar Dec 06 '20 12:12 xhebox