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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ts_project] using Angular compiler with composite = True errors because ngc doesn't generate .tsbuildinfo #2885

Closed
matthewjh opened this issue Aug 24, 2021 · 4 comments
Labels
Can Close? We will close this in 30 days if there is no further activity

Comments

@matthewjh
Copy link
Contributor

馃悶 bug report

Affected Rule

ts_project

Is this a regression?

n/a

Description

In ts_project, the .tsbuildinfo file is declared as an output so that bazel will delete the file when re-building the rule. However, ngc does not generate .tsbuildinfo files because it does not support tsc incremental mode:

angular/angular#42011

Passing ts_build_info_file = None as an attribute on the ts_project rule doesn't work, as the implementation defaults the downstream attribute:

tsbuildinfo_path = ts_build_info_file if ts_build_info_file else name + ".tsbuildinfo"

My current workaround is to patch ts_project.bzl to comment out:

buildinfo_out = tsbuildinfo_path if composite or incremental else None,

and replace with

buildinfo_out = None

but this is too blunt an instrument and will cause bazel not to delete .tsbuildinfo files when it would be present e.g,. through non tsc = ngc rules.

I'm not sure what the best way to fix this would be. Seems we more or less want

buildinfo_out = if tsc is ngc then None else regular_buildinfo_out

But this means encoding the ngc node_binary target into ts_project, which seems nasty.

A cleaner alternative is to add a new attribute force_no_tsbuildinfo_path.

If someone who knows more about this codebase and best practices can lend some advice, I am happy to take a stab at implementing a fix.

馃敩 Minimal Reproduction

https://github.com/matthewjh/rules_nodejs/tree/ngc_tsbuildinfo_error_repo

bazel build //packages/typescript/test/ts_project/a:tsconfig

馃敟 Exception or Error



INFO: Analyzed target //packages/typescript/test/ts_project/a:tsconfig (1 packages loaded, 5 targets configured).
INFO: Found 1 target...
ERROR: C:/users/matt/documents/github/rules_nodejs_fork/rules_nodejs/packages/typescript/test/ts_project/a/BUILD.bazel:12:11: output 'packages/typescript/test/ts_project/a/tsconfig.tsbuildinfo' was not created
ERROR: C:/users/matt/documents/github/rules_nodejs_fork/rules_nodejs/packages/typescript/test/ts_project/a/BUILD.bazel:12:11: Compiling TypeScript project //packages/typescript/test/ts_project/a:tsconfig [tsc -p packages/typescript/test/ts_project/a/tsconfig.json] failed: not all outputs were created or valid
Target //packages/typescript/test/ts_project/a:tsconfig failed to build
Use --verbose_failures to see the command lines of failed build steps.


馃實 Your Environment

Operating System:

  
  Windows 10
  

Output of bazel version:

  
 Bazelisk version: v1.5.0
Build label: 4.1.0
Build target: bazel-out/x64_windows-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Fri May 21 11:17:01 2021 (1621595821)
Build timestamp: 1621595821
Build timestamp as int: 1621595821

  

Rules_nodejs version:

(Please check that you have matching versions between WORKSPACE file and @bazel/* npm packages.)

  
 stable
  
@matthewjh
Copy link
Contributor Author

matthewjh commented Sep 6, 2021

@alexeagle any preference or thought on how I can handle this cleanly? so I can raise a PR and hopefully get this fix in the next release.

I think ideally the tsc value itself would convey whether it generates a tsbuildinfo file or not (tsc = {binary, generates_tsbuildinfo}), but I'm not sure whether such as thing is even possible and convenient in bazel.

@alexeagle
Copy link
Collaborator

What's the benefit of setting composite = True on an ngc compilation with ts_project? Does removing that fix it?

I'm also curious if you can set tsBuildInfoFile in the tsconfig to some sentinel value like empty string, would that suppress typescript creating it? Then ts_project could also ignore empty values.

@github-actions
Copy link

github-actions bot commented Dec 6, 2021

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Dec 6, 2021
@github-actions
Copy link

github-actions bot commented Jan 5, 2022

This issue was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

@github-actions github-actions bot closed this as completed Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? We will close this in 30 days if there is no further activity
Projects
None yet
Development

No branches or pull requests

2 participants