fix(compiler-cli): re-tag SourceFiles after TsCreateProgramDriver.updateFiles()#69548
fix(compiler-cli): re-tag SourceFiles after TsCreateProgramDriver.updateFiles()#69548LordKay-sudo wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
f36d951 to
7c17dc5
Compare
|
CI was failing because the test imported \isExtended\ from ../../shims, but that symbol isn't part of the public shims API. Dropped the import in 9e96059. The test still checks that |
9e96059 to
f889376
Compare
JoostK
left a comment
There was a problem hiding this comment.
Just noting here that I'm aware of this PR; it's just that I need to look into this proper; I'm not certain that tagging the main TS program won't introduce problems w.r.t dangling .ngtypecheck.ts references in the program that's used for emit.
|
Thanks for looking at this, @JoostK . Fair point on the emit side. The Emit still goes through Pushed two follow-up tests in d8927ca:
|
JoostK
left a comment
There was a problem hiding this comment.
Thanks for the update. There's a few remaining problems:
- The tests do not compile; fixing that reveals a failing test.
- When uncommenting the length assertions to allow the
getSemanticDiagnosticscheck to be exercised, no tests fail after uncommenting the fix.
Please also squash all commits into a single fix commit, or use fixup! commits if you want to retain the commit fidelity in the PR (will be autosquashed when merged)
…ateFiles() TypeScript reuses SourceFile objects between old and new programs, so untagging the old program also untags shared files in the new program. Re-apply shim tags on the new program to prevent getSemanticDiagnostics() crashes with TS 5.5+. Adds regression tests that update shim files only so shared SourceFiles remain shared, and verifies both diagnostic safety and emit-time untagging. Fixes angular#68164
d8927ca to
9d67100
Compare
|
@JoostK Thanks for the review. Pushed a squashed fix in 9d67100. On the tests:
The length assertion and |
Summary
retagAllTsFiles(this.program)afteruntagAllTsFiles(oldProgram)inTsCreateProgramDriver.updateFiles().SourceFileobjects between old and new programs, so untagging the old program also untags shared files in the new program. Without re-tagging,getSemanticDiagnostics()can crash on TS 5.9+ withCannot destructure property 'pos' of 'file.referencedFiles[index]'.SourceFiles afterupdateFiles().Fixes #68164
Test plan
referencedFiles.lengthdrops belowtaggedReferenceFiles.lengthafterupdateFiles()(sharedSourceFileuntagged).referencedFilesmatchestaggedReferenceFilesandgetSemanticDiagnostics()completes without throwing.bazel test //packages/compiler-cli/src/ngtsc/typecheck/test:test(could not run Bazel locally on Windows due to bash genrule failure; CI should validate)