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

Call all plugins onResolve instead of using only the first one #501

Open
remorses opened this issue Nov 1, 2020 · 3 comments
Open

Call all plugins onResolve instead of using only the first one #501

remorses opened this issue Nov 1, 2020 · 3 comments
Labels

Comments

@remorses
Copy link
Contributor

remorses commented Nov 1, 2020

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 remorses changed the title Call all plugins resolvers instead of using only the first one Call all plugins onResolve instead of using only the first one Nov 1, 2020
@Jarred-Sumner
Copy link
Contributor

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

@remorses
Copy link
Contributor Author

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

@xhebox
Copy link

xhebox commented Dec 6, 2020

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 #546 (comment). 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants