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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad cache when type checking project references #48981

Closed
samreid opened this issue May 6, 2022 · 2 comments
Closed

Bad cache when type checking project references #48981

samreid opened this issue May 6, 2022 · 2 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@samreid
Copy link

samreid commented May 6, 2022

Bug Report

🔎 Search Terms

Cache, build, project references, stale, bad

🕗 Version & Regression Information

  • I was unable to test this on prior versions because the problem is spurious and difficult to reproduce. But our team thinks we have seen this back around 4.5 and this report occurred in 4.6.

💻 Code

As reported in phetsims/chipper#1243 (comment), we invoked the type checker via node ../chipper/node_modules/typescript/bin/tsc -b. We expected to see no type errors.

🙁 Actual behavior

The result of running the command above was unexpected type errors:

../vegas/js/demo/FiniteChallengesScreenView.ts:69:20 - error TS2345: Argument of type 'FiniteStatusBar' is not assignable to parameter of type 'Node'.
  Type 'FiniteStatusBar' is missing the following properties from type 'Node': _id, _instances, _rootedDisplays, _drawables, and 631 more.

...
Found 8 errors.

🙂 Expected behavior

After --clean the type checking passed as expected:

$ node ../chipper/node_modules/typescript/bin/tsc -b

../vegas/js/demo/FiniteChallengesScreenView.ts:69:20 - error TS2345: Argument of type 'FiniteStatusBar' is not assignable to parameter of type 'Node'.
  Type 'FiniteStatusBar' is missing the following properties from type 'Node': _id, _instances, _rootedDisplays, _drawables, and 631 more.

...
Found 8 errors.

$ node ../chipper/node_modules/typescript/bin/tsc -b --clean
$ node ../chipper/node_modules/typescript/bin/tsc -b
$ 

This indicates an inconsistency or incorrectness in the project references build cache. The only step that we took between "8 type errors" and 0 type errors was running --clean. To us, this indicates a stale or bad cache.

Questions

  • Are there other known/documented cache problems for project references?
  • Can this symptom occur for a single project (no project references) but using incremental: true?
  • Can a bad cache produce both false positives (reports a type error that doesn't exist) and a false negative (reporting success when there is actually a type error)? This report is a false positive only. My team thinks they recall false negatives, but we don't have an exact error trace of that occurring.
  • Is this problem related to tsc -b incorrectly succeeds when there is a problem introduced by a transitive dependency #46153?
  • If our team doesn't want to have false positives or false negatives any more, is the best remedy to --clean every single time we type check?
@RyanCavanaugh
Copy link
Member

As a workaround, you can run tsc -b --force instead of a separate --clean step.

There is no separate "cache" per se. The build depends only on inputs/outputs from your project itself. That said, internal state during one build can leak into the next build due to bugs.

Your list of questions is sort of difficult for me to answer since you're asking about behavior that would manifest from bugs which, by definition, produce unexpected results. It seems like you're encountering a bug (incremental and nonincremental builds are not supposed to have different error arity) so I can't answer what the effects of that bug could/couldn't be.

Is there a way we can reproduce the issue locally?

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label May 6, 2022
@samreid
Copy link
Author

samreid commented May 7, 2022

This issue seems to have the same cause as #46153. In that issue, @sheetalkamat indicated that each tsconfig should include imported dependencies in its array of references. Our project has been relying on transitive dependencies--if projectA imports C from projectC, we thought it was sufficient for projectA to have projectB in its references as long as projectB has projectC in its references. In phetsims/chipper#1245, we will try listing all of the imported dependencies explicitly and see if this kind of error disappears.

I don't see explicit mention of this requirement in https://www.typescriptlang.org/docs/handbook/project-references.html, but may be good to mention if it will help teams that have project structures like ours. (Or maybe it is already mentioned or implied in a way that I didn't follow.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

2 participants