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

Plugins don't seem to apply to entrypoints #546

Closed
jbms opened this issue Nov 19, 2020 · 10 comments
Closed

Plugins don't seem to apply to entrypoints #546

jbms opened this issue Nov 19, 2020 · 10 comments

Comments

@jbms
Copy link

jbms commented Nov 19, 2020

I thought I could use a plugin that would map entry points to an auto-generated javascript source containing a list of import statements, as a way to bundle multiple "entry" source files into a single bundle.

However, the plugin was never called on the entrypoint paths.

@evanw
Copy link
Owner

evanw commented Nov 19, 2020

I just looked into doing this. This is potentially a breaking change for these reasons:

  • Plugins that currently don't match the entry point will now match it. That's the point of course, but it's still a change in behavior. It can lead to unexpected results because entry point paths are different than normal paths (more about this below).

  • Entry point paths don't have an importer. Plugins may not be expecting this and could crash if they get a blank importer path. Although maybe this isn't something to worry about?

  • This is the big one. Entry point paths currently behave differently than import paths. First of all, they require you to specify the extension instead of allowing the extension to be omitted. They are also automatically treated as relative even when they don't start with ./. import paths must start with ./ to be treated as relative, otherwise they are treated as a package path within the nearest node_modules folder (following node's module resolution algorithm).

I'm not quite sure what to do about the last part. Some options:

  1. Make entry point paths no longer special. This means entry point paths not starting with ./ would now be considered package paths instead of relative paths. Change esbuild to require entry point paths to start with ./ to be resolved as relative paths. I can't think of a time when I've ever wanted this, so it would make using esbuild more annoying.

    I noticed that Webpack 5 actually behaves like this now (you must use ./ for relative entry points). Webpack 4 behaved like esbuild does now (you can omit ./ for relative entry points). I wonder what the thought process was for switching. Edit: I found some related issues, although they didn't answer my question: Different requirements for entry and output.path configuration options. webpack/webpack#1908 and Webpack 4 entry point requires leading ./ on Windows webpack/webpack#7919.

  2. Special-case esbuild's default path resolution behavior to always automatically insert ./ only for entry point paths. This would keep the user-facing behavior the same as it is today. Plugins would still get the verbatim entry point path from the user without the leading ./. This could make writing plugins more annoying since there would be an additional special case to handle. You could tell if something is an entry point path if the importer and the namespace are both empty strings.

  3. Automatically insert ./ for all entry point paths, even when they are passed to plugins. This would behave the way it does today and would mean most plugins wouldn't need any changes. However, it would mean plugins that specifically want to match on entry points that the user typed in would have to anticipate the additional ./ being inserted, which could be confusing for plugin authors.

@jbms
Copy link
Author

jbms commented Nov 19, 2020

Thanks for looking into this. One simple solution could be to add an option, defaulting to true, to insert "./" for all entry point paths. Alternatively, option 3 seems like it would be reasonable, and could be documented so as to avoid confusion.

@ggoodman
Copy link

ggoodman commented Nov 20, 2020

@evanw have you considered the following other options:

  1. Add a distinct plugin hook (ie: build.onResolveEntrypoint()) for entrypoint resolution that will only be invoked for resolving entrypoints. This would allow the arguments to onResolve to remain consistent across scenarios and would provide an easy mechanism for plugin authors to opt-in for this more nuanced scenario.

  2. Add a new flag to the Resolve Options that would allow plugins to opt-in to entrypoint resolution. If that new flag were non-boolean, the question of ./ could be avoided by allowing plugin authors to chose which approach to take. For example:

    build.onResolve({ entrypoints: 'prefixed', filter: /./, onResolveHandler);

@rtsao
Copy link
Contributor

rtsao commented Nov 20, 2020

I have to admit I was a bit surprised to learn that onResolve is not invoked for entry files. It makes sense now, but it definitely ran contrary to my expectations (influenced by webpack no doubt).

A couple thoughts:

Make entry point paths no longer special. ...it would make using esbuild more annoying.

If I'm already using a JS file to configure and run esbuild, it hardly seems like much burden to specify entryPoints: ["./src/index.js"] instead of entryPoints: ["src/index.js"]. In fact, it seems I was already doing this in the first place. On the other hand, I can definitely see how enforced relative path prefixes would be quite annoying when using the esbuild CLI.

However, it would mean plugins that specifically want to match on entry points that the user typed in would have to anticipate the additional ./ being inserted, which could be confusing for plugin authors.

I think it's somewhat expected that comparing non-absolute paths will be inherently brittle. And certainly third-party plugins can't make assumptions regarding the entryPoints. So perhaps esbuild could explicitly provide entry: true as part of the onResolve context.

Going back to the prior point, it seems like esbuild could safely insert ./ for all entry points only for CLI usage. This addresses the annoyance factor for simple usage and then there's practically zero risk of some plugin programmatically expecting to see the unprefixed src/index.js in onResolve because there's not really a convenient mechanism for sharing variables between the CLI invocation and the plugin source code. In fact, is it even possible to use plugins when using the CLI?

@lukeed
Copy link
Contributor

lukeed commented Nov 22, 2020

A cheeky way to get around this right now (via API) is to map the contents of entryPoints into a throwaway src/entry.js file, then use src/entry.js as the entry. For example:

// src/entry.js
import('/path/to/item1.js');
import('/path/to/item2.js');
import('/path/to/item3.js');
esbuild.build({
  entryPoints: ['src/entry.js'],
  // ...
});

Now all original entries (itemX.js) can pass through plugins.
Your src/entry.js can reside within /tmp if desired, and you can wade thru metafile output to delete its destination file.

for (let file in metafile.outputs) {
  if ('src/entry.js' in metafile.outputs[file].inputs) {
    fs.unlink(file);
  }
}

@ggoodman
Copy link

Thanks for the idea @lukeed that does allow me to unblock some ideas. What I feel is still missing is that this approach precludes real code-splitting. I'm sure we'll get there pretty soon!

Unfortunately, I'm a total golang noob so my effort to introduce a sort of inputFilesystem option fell flat on its face. The idea would have been to seed the disk cache with some custom files. That way, I was hoping entrypoints would just work even if the files weren't really on disk.

@evanw
Copy link
Owner

evanw commented Nov 23, 2020

In fact, is it even possible to use plugins when using the CLI?

That's a good point. This is currently not possible, although it may be possible in the future. There are some ideas for doing this here: #111 (comment).

Going back to the prior point, it seems like esbuild could safely insert ./ for all entry points only for CLI usage.

This is interesting. I think it should work in the vast majority of cases, and it's very simple. It wouldn't let you transparently do things like esbuild http://... --plugin:./http-plugin at some point in the future because the plugin would see ./http://... but maybe that's ok since you can just use the API for that. Or maybe esbuild could not do this for :// paths. Or plugins could opt-in to raw entry path strings. Anyway, there are ways to solve that and it's a rare edge case. This is a breaking change for code that uses the API though so I'd want to wait until the next round of breaking changes.

@remorses
Copy link
Contributor

remorses commented Dec 1, 2020

I really need this for the project I am working on: https://github.com/remorses/vite-esbuild-optimizer

I need to bundle node_modules entries and it's impossible to resolve the entrpoints when using Yarn PnP because these should pass trough the onResolve plugin first

I hope this will land soon 🙏

@evanw
Copy link
Owner

evanw commented Dec 8, 2020

I'm going to land this as a non-breaking change to avoid waiting longer. The ./ insertion will be done for both the CLI and the API, but only if the file at that relative path actually exists.

@evanw evanw closed this as completed in 05eaca4 Dec 8, 2020
@evanw
Copy link
Owner

evanw commented Dec 8, 2020

This has been fixed in version 0.8.21.

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

No branches or pull requests

6 participants