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

deno check cache causes wrong results #18516

Closed
NfNitLoop opened this issue Mar 31, 2023 · 5 comments · Fixed by #18541
Closed

deno check cache causes wrong results #18516

NfNitLoop opened this issue Mar 31, 2023 · 5 comments · Fixed by #18541
Assignees
Labels
bug Something isn't working

Comments

@NfNitLoop
Copy link

Summary

The cache used by deno check can lead to incorrect results reported. Running deno check app.ts a second time will just return the previous result, even though the types of local modules that app.ts depends on have been corrected.

Reproduction

Given the files:

// File: app.ts
import { greet } from "./_greet.ts"

greet("world")

and:

// File: _greet.ts
export function greet(name: number) {
    console.log(`Hello, ${name}!`)
}

deno check gives the expected error:

Check file:///C:/Users/Cody/code/deno/check-cache-bug/app.ts
error: TS2345 [ERROR]: Argument of type 'string' is not assignable to parameter of type 'number'.
greet("world")
      ~~~~~~~
    at file:///C:/Users/Cody/code/deno/check-cache-bug/app.ts:4:7

If we fix _greet.ts to take a name: string, deno check should now pass, but we still get the same error.

You can work around this by running deno check --reload app.ts, but there are two big problems with this:

First: For large projects, this can result in re-downloading quite a large dependency tree with every run of deno check --reload.

But more importantly, you have to first know that you're getting the wrong results to know to go look for --reload. If you run into the caching problem from the other direction (i.e.: you cached a "pass", which should now be failing), deno check is giving you a dangerous result. You think your build is good and you push it out, only to run into runtime errors that TypeScript should've saved you from.

Possible fixes?

Maybe when deno check caches its results, it can save the graph of local dependencies checked. Then, the next deno check will only use the cache if none of those local dependencies have changed.

@nayeemrmn
Copy link
Collaborator

This looks like a TS 5.0 buildinfo bug. cc @dsherret

@dsherret dsherret added the bug Something isn't working label Mar 31, 2023
@dsherret dsherret self-assigned this Mar 31, 2023
@dsherret
Copy link
Member

You can temporarily work around this by providing the --all flag

@NfNitLoop
Copy link
Author

--all isn't an exact work-around though. It changes the scope of the check. It can cause errors with type checking in remote code (which a user is likely not able to fix), even though all your local types are correct. I think --reload works around this w/o that scope change, albeit at the cost of constantly re-downloading dependencies.

I guess for folks running into this, try --all, which will be the "cheaper" workaround if it works, and if it doesn't, fall back to --reload.

@dsherret
Copy link
Member

dsherret commented Apr 1, 2023

This is a regression caused by #18329. Luckily there is a fix in #18541 that still keeps things fast.

We didn't have a test for this. Now we have a test for it so it shouldn't happen again in the future. Sorry about that!

dsherret added a commit that referenced this issue Apr 1, 2023
…nvalidated between runs (#18541)

Regression caused by the performance improvement in #18329. Figuring
this out was hard. It's luckily still fast after this change.

Closes #18516
dsherret added a commit that referenced this issue Apr 1, 2023
…nvalidated between runs (#18541)

Regression caused by the performance improvement in #18329. Figuring
this out was hard. It's luckily still fast after this change.

Closes #18516
@dsherret
Copy link
Member

dsherret commented Apr 2, 2023

Should be fixed now in 1.32.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants