-
Notifications
You must be signed in to change notification settings - Fork 522
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
feat: augment TS2307 diagnostic message with missing deps info #1994
Conversation
ctx, | ||
progress_message = "Checking TSC Diagnostics", | ||
mnemonic = "tscdiag", | ||
# outputs = action_outputs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexeagle This is the part that's tripping me up. On a failed compile, the tsc action is forced to exit with 0 so the following action can run and inspect stderr, but it still declares it's outputs that aren't created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand, which action declares outputs that aren't created? tsc or the diagnostic processor? maybe an error message would help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsc, it produces the following when the exit code is forced 0 with missing deps (add import * as fs from 'fs';
to packages/typescript/test/strict_deps/grandparent.ts
)
bazel build //packages/typescript/test/strict_deps:grandparent
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 7d2cbd7e-e9e7-459f-8c1c-fc02ca1d6eb1
INFO: Analyzed target //packages/typescript/test/strict_deps:grandparent (53 packages loaded, 878 targets configured).
INFO: Found 1 target...
INFO: Deleting stale sandbox base /private/var/tmp/_bazel_matt/4afd3aeed021b99395b4ccda14f35222/sandbox
ERROR: /Users/matt/Documents/workspace/rules_nodejs/packages/typescript/test/strict_deps/BUILD:17:1: output 'packages/typescript/test/strict_deps/grandparent.js' was not created
ERROR: /Users/matt/Documents/workspace/rules_nodejs/packages/typescript/test/strict_deps/BUILD:17:1: output 'packages/typescript/test/strict_deps/grandparent.d.ts' was not created
ERROR: /Users/matt/Documents/workspace/rules_nodejs/packages/typescript/test/strict_deps/BUILD:17:1: not all outputs were created or valid
Target //packages/typescript/test/strict_deps:grandparent failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 16.734s, Critical Path: 2.30s
INFO: 1 process: 1 darwin-sandbox.
FAILED: Build did NOT complete successfully
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, right. I hadn't realized this flaw in the design. It needs to be run with --emitOnError
if we want those outputs to be produced on failure, which I think is the only choice here, since we have to predict the outputs to Bazel before knowing whether the action will succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately even after forcing noEmitOnError
to false (it's set true by tsconfig.bzl), tsc_wrapped exits early if there are any diagnostics before emitting, so if we wanted to carry on here, I feel we'd need to work around that somehow, and it all starts feeling a bit messy?
This approach may still work with ts_project, I haven't tried that yet.
|
||
process.stderr.write(tsDiagnostics); | ||
|
||
process.exit(parseInt(exitcode, 10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is this better than Number(exitcode)
?
ctx, | ||
progress_message = "Checking TSC Diagnostics", | ||
mnemonic = "tscdiag", | ||
# outputs = action_outputs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand, which action declares outputs that aren't created? tsc or the diagnostic processor? maybe an error message would help.
Augments TS2307 with additional info when a dependency is missing from the
deps
attr and compilation unit.This was branched from #1990