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

fix(esbuild): prefer finding entry_point files in deps rather than srcs #2692

Merged

Conversation

mattem
Copy link
Collaborator

@mattem mattem commented May 24, 2021

When using esbuild with js_library, the entry_point may be part of the deps, therefore the path will end up in bazel-out/ inside the sandbox where esbuild is running, however the entrypoint path will be in the sources path, leaving any relative path inputs to no longer be relative to the entrypoint file.

The reordering here forces the resolve_entry_point method to prefer the file that resides in the bazel-out/ path in the sandbox.

A possible future breaking change would be to force users to list the entrypoint file(s) in either srcs or deps for the correct pathing, or remove srcs from esbuild, meaning all input files must be in the output tree.

When using esbuild with `js_library`, the entry_point may be part of the deps, therefore the path will end up in `bazel-out/` inside the sandbox where esbuild is running, however the entrypoint path will be in the sources path, leaving any relative path inputs to no longer be relative to the entrypoint file.

The reordering here forces the `resolve_entry_point` method to prefer the file that resides in the `bazel-out/` path in the sandbox.

A possible future breaking change would be to force users to list the entrypoint file(s) in either srcs or deps for the correct pathing, or remove `srcs` from esbuild, meaning all input files must be in the output tree.
@jbedard
Copy link
Collaborator

jbedard commented May 24, 2021

A possible future breaking change would be to force users to list the entrypoint file(s) in either srcs or deps for the correct pathing, or remove srcs from esbuild, meaning all input files must be in the output tree.

👍 - I often find it confusing when a bundle-type rule has both srcs and deps. I would general lean towards deps but 🤷

@mattem
Copy link
Collaborator Author

mattem commented May 25, 2021

Yeah I agree it can be confusing. The intention was to support where esbuild can take ts sources directly, these would go in srcs, rather than via a something and into deps. There is an example of this in the tests, but no idea if anyone holds it this way (I guess we have to assume they do).

I'd also like to support using esbuild without the --bundle flag, passing in TS files and having it output JS (currently each input file has to be listed explicitly as an entry point, esbuild won't just output JS from a single TS entry)

@joneshf
Copy link

joneshf commented May 25, 2021

A minor hiccup with removing srcs, is that there'd have to be some alternative way to inject a file. What I currently do (not sure if it's the right way) is add the file to srcs and then add an entry to the args attribute: "--inject:$(execpath process-shim.js)".

I assume that is srcs was removed, the rule would need to grow an inject attribute that could take files and deal with them accordingly.

@alexeagle
Copy link
Collaborator

@joneshf I don't think anything would change for you under that proposal other than anything you have in srcs you would move to deps

@alexeagle alexeagle merged commit dd4c4f3 into bazel-contrib:stable May 25, 2021
@mattem mattem deleted the fix/esbuild_js_library_entrypoint branch May 26, 2021 00:25
alexeagle pushed a commit that referenced this pull request May 26, 2021
…cs (#2692)

When using esbuild with `js_library`, the entry_point may be part of the deps, therefore the path will end up in `bazel-out/` inside the sandbox where esbuild is running, however the entrypoint path will be in the sources path, leaving any relative path inputs to no longer be relative to the entrypoint file.

The reordering here forces the `resolve_entry_point` method to prefer the file that resides in the `bazel-out/` path in the sandbox.

A possible future breaking change would be to force users to list the entrypoint file(s) in either srcs or deps for the correct pathing, or remove `srcs` from esbuild, meaning all input files must be in the output tree.
twheys pushed a commit to twheys/rules_nodejs that referenced this pull request Jan 13, 2022
…cs (bazel-contrib#2692)

When using esbuild with `js_library`, the entry_point may be part of the deps, therefore the path will end up in `bazel-out/` inside the sandbox where esbuild is running, however the entrypoint path will be in the sources path, leaving any relative path inputs to no longer be relative to the entrypoint file.

The reordering here forces the `resolve_entry_point` method to prefer the file that resides in the `bazel-out/` path in the sandbox.

A possible future breaking change would be to force users to list the entrypoint file(s) in either srcs or deps for the correct pathing, or remove `srcs` from esbuild, meaning all input files must be in the output tree.
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 this pull request may close these issues.

4 participants