fix: Candidate fix for https://github.com/aspect-build/rules_esbuild/issue…

[JonathanPerry651]

Candidate fix for #251.

Changes are visible to end-users: yes

• Searched for relevant documentation and updated as needed: no
• Breaking change (forces users to change their own code or config): no
• Suggested release notes appear below: no

Test plan

• New test cases added

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[aspect-workflows]

Test (Bazel 7) (Test)

All tests were cache hits

18 tests (100.0%) were fully cached saving 1s.

---

Test (Bazel 8) (Test)

All tests were cache hits

18 tests (100.0%) were fully cached saving 849ms.

---

Test (Bazel 7) (Test)

e2e/bundle

All tests were cache hits

2 tests (100.0%) were fully cached saving 96ms.

---

Test (Bazel 8) (Test)

e2e/bundle

All tests were cache hits

2 tests (100.0%) were fully cached saving 110ms.

---

Test (Bazel 7) (Test)

e2e/custom_version

All tests were cache hits

2 tests (100.0%) were fully cached saving 156ms.

---

Test (Bazel 8) (Test)

e2e/custom_version

All tests were cache hits

2 tests (100.0%) were fully cached saving 161ms.

---

Test (Bazel 7) (Test)

e2e/npm-links

All tests were cache hits

2 tests (100.0%) were fully cached saving 107ms.

---

Test (Bazel 8) (Test)

e2e/npm-links

All tests were cache hits

2 tests (100.0%) were fully cached saving 94ms.

---

Test (Bazel 7) (Test)

e2e/smoke

All tests were cache hits

1 test (100.0%) was fully cached saving 31ms.

---

Test (Bazel 8) (Test)

e2e/smoke

All tests were cache hits

1 test (100.0%) was fully cached saving 26ms.

---

Test (Bazel 7) (Test)

e2e/sourcemaps

All tests were cache hits

7 tests (100.0%) were fully cached saving 313ms.

---

Test (Bazel 8) (Test)

e2e/sourcemaps

All tests were cache hits

7 tests (100.0%) were fully cached saving 335ms.

---

Test (Bazel 7) (Test)

e2e/toolchain_from_source

All tests were cache hits

---

Test (Bazel 8) (Test)

e2e/toolchain_from_source

All tests were cache hits

---

Test (Bazel 7) (Test)

e2e/tsconfig

All tests were cache hits

3 tests (100.0%) were fully cached saving 135ms.

---

Test (Bazel 8) (Test)

e2e/tsconfig

All tests were cache hits

3 tests (100.0%) were fully cached saving 165ms.

---

Buildifier

[alexeagle]

Yes it's commonly the case when (.js,.ts) file pairs are accidentally provided as inputs, that the developer didn't intend for the .ts files to be there. If they have a transpilation setup in their ts_project (not emitDeclarationOnly) then they probably meant to use the transpiled files.

But it's not clear to me that all users would want this behavior.

If esbuild is configured with a plugin that transpiles TS files, it's possible they intended that plugin should get the path/to/some.ts file in the execroot, even though path/to/some.js is provided there as well.

@jbedard what do you think - is that a remote possibility that warrants making this configurable?

[jbedard]

@JonathanPerry651 do you understand why the .ts file is available at all? Ideally sandboxing would prevent that file from even being accessible to esbuild. Do you have sandboxing off?

[jbedard]

@jbedard what do you think - is that a remote possibility that warrants making this configurable?

I think we can probably merge this as-is and maybe allow opt-out if we run into issues. My main concern would be performance, not lost functionality.