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

esbuild rule is incompatible with bundling from .ts sources #80

Open
mrmeku opened this issue Oct 4, 2022 · 2 comments
Open

esbuild rule is incompatible with bundling from .ts sources #80

mrmeku opened this issue Oct 4, 2022 · 2 comments
Labels
enhancement New feature or request
Milestone

Comments

@mrmeku
Copy link

mrmeku commented Oct 4, 2022

Throughout the code base of rules_esbuild there are multiple places which make it impractical if not impossible to use a typescript entry point and have esbuild resolve depedencies using a tsconfig.json file.

To start this is this line which explictly removes the ability to pass esbuild a tsconfig file:

Secondly, when gathering transitive input sources, only files from js_providers are considered and the rest are thrown out: https://github.com/aspect-build/rules_esbuild/blob/main/esbuild/private/esbuild.bzl#L301

This is problematic since it leaves no way for targets to specify runtime dependencies they want to make available to esbuild during bundling. A common use case for this is to have a css import in a .ts or .js file import ./index.css. One would make index.css a data dependency of their ts_project or js_library target, expecting that this would make it so that the esbuild bundler would make the file available during bundling. However, this is not the case.

The only way to include non .js files is to specify the entire transitive closure of targets you need available within the srcs attribute. This is not sufficient for builds with many transitive dependencies. Each transitive dep should be able to declare what it needs to have available during bundling.

@cgrindel cgrindel added the enhancement New feature or request label Oct 5, 2022
@cgrindel
Copy link
Contributor

@jbedard will bring in additional context from a Slack thread with the author of the issue.

@jbedard
Copy link
Member

jbedard commented Oct 13, 2022

@mrmeku if the line you pointed out is deleted can you use js_library for the full dep tree?

Then for the type-checking ts_project(declaration_only=True,testonly=True) + build_test. I'm not 100% sure if that ts_project could then depend on the js_library (leaf nodes of the js_library dep tree) or if it would have to depend on the other ts_projects (parallel dep tree).

@gregmagolan gregmagolan added this to the 1.0 milestone Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: On Deck
Development

No branches or pull requests

4 participants