[Bug]: copy_data_files_to_bin (for gather_runfiles) depends on whether transpiler is set

[gzm0]

What happened?

It seems that ts_project doesn't expose this config option yet and the defaults are different.

Without transpiler

1. data is passed to the ts_project rule here:
   

   rules_ts/ts/defs.bzl

   Lines 406 to 412 in 42094e8

   	ts_project_rule(
   	name = tsc_target_name,
   	srcs = srcs,
   	args = args,
   	assets = assets,
   	data = data,
   	deps = tsc_deps,

2. The ts_project rule invokes gather_runfiles
   

   rules_ts/ts/private/ts_project.bzl

   Lines 270 to 275 in 42094e8

   	runfiles = js_lib_helpers.gather_runfiles(
   	ctx = ctx,
   	sources = output_sources_depset,
   	data = ctx.attr.data,
   	deps = ctx.attr.srcs + [ctx.attr.tsconfig] + ctx.attr.deps,
   	)

3. The default for copy_data_files_to_bin is False

With transpiler

1. data is passed to js_library here:
   

   rules_ts/ts/defs.bzl

   Lines 392 to 400 in 42094e8

   	js_library(
   	name = name,
   	# Include the tsc target in srcs to pick-up both the direct & transitive declaration outputs so
   	# that this js_library can be a valid dep for downstream ts_project or other rules_js derivative rules.
   	srcs = [transpile_target_name, tsc_target_name] + assets,
   	deps = deps,
   	data = data,
   	**common_kwargs
   	)

2. The default for js_library is True

What I'd expect

• The behavior with and without transpiler to be the same.
• There be an equivalent option to js_library on ts_project (the macro).

Version

Development (host) and target OS/architectures:

Output of bazel --version:

bazelisk --version
bazel 6.3.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file:

rules_js: 1.31.0
rules_ts: 1.4.5

Language(s) and/or frameworks involved: None.

How to reproduce

Minimal steps, happy to provide more if necessary.

1. Add a filegroup with files in a different package to `data` of a `ts_project` (without `transpiler` set).
2. Build passes.
3. Set `transpiler` to `swc`.
4. Build fails (example failure below)
5. Switch `filegroup` to `js_library`
6. Build passes



Expected to find source file <the file> in '//package-of-ts-project', but instead it is in '//package-of-file'.

All source and data files that are not in the Bazel output tree must be in the same package as the
target so that they can be copied to the output tree in an action.

See https://docs.aspect.build/rules/aspect_rules_js/docs/#javascript for more context on why this is required.

### Any other information?

Happy to open a PR to align this, but I'd like to agree on the following first:
- Exact fix (add flag, align behavior, ???)
- Target branch (1.x, 2.x)?

[alexeagle]

The behavior with and without transpiler to be the same

I don't think we make this promise anywhere, though I can understand why it's unfortunate to make non-local changes when switching the transplier

There be an equivalent option to js_library on ts_project (the macro).

Is this just saying you need a boolean attribute to be exposed in the public API to control copy_data_files_to_bin?

It would be really great if you could find somewhere in our examples/e2e folder where you could reproduce the problem caused by your data files getting copied or not copied, and how the runfiles is involved, so we could study a bit more.

[gzm0]

I don't think we make this promise anywhere, though I can understand why it's unfortunate to make non-local changes when switching the transplier

Fair enough. Although this one was particularly confusing to me because I'd expect the data attribute not to interact with the compilation/transpilation step at all (but only with downstream execution steps).

Is this just saying you need a boolean attribute to be exposed in the public API to control copy_data_files_to_bin?

Yes, pretty much (and maybe, but IIUC for 2.0.0 it's already too late, make it the same default, independent of whether transpiler is set).

It would be really great if you could find somewhere in our examples/e2e folder where you could reproduce the problem caused by your data files getting copied or not copied, and how the runfiles is involved, so we could study a bit more.

TBH, I do not understand the specific effect this has on the runfiles tree (I shamefully admit that when it comes to runfiles, I just fiddle with relative paths until my code can find the files).

The way I hit this issue is by depending on a filegroup:

ts_project(
  name = "test",
  // stuff
  data = [
    "//other-package:files" // <-- filegroup target containing source files
  ],
)

copy_to_bin will fail on //other-package:files, because it can only act on files in the invoking package (so for all I know the resulting runfiles tree would look the same).

To me, it feels this is reason enough to add the copy_data_files_to_bin options to ts_project but if you feel this needs some deeper understanding of how this actually affects runfiles (beyond what is documented in rules_js), I'm happy to do some digging.

[gzm0]

FWIW: if #717 get's accepted, all that needs to be done to fix this is remove

rules_ts/ts/defs.bzl

Lines 428 to 429 in 6de1bc2

	if copy_data_to_bin == None:
	copy_data_to_bin = transpiler != None

and set a normal default value for copy_data_to_bin

The tests in https://github.com/aspect-build/rules_ts/blob/6de1bc21b013988fe7476240778f27c5309d75ee/ts/test/copy_data_to_bin/BUILD.bazel will fail and need to be adjusted.