fix(cache): invalidate codeCache in most cases when imports change
#369
Conversation
- previously, the `checkImports` flag was set to `true` for type-checking, but `false` for compilation
- I _believe_ it is `false` because the compiled JS shouldn't change if an import changes
- though I'm not sure if that was the original intent behind the code
- problematically though, compilation results can include declarations, and those _can_ change if imports change
- for instance, the types of an import can change the declaration that is output
- so now, only set it to `false` for compilation if declarations are _not_ needed
agilgur5
added
kind: bug
|
I don't remember why it is this way, but I think your change is correct. Now that I look at it closer, maybe we should always check imports regardless -- there might be cases where typescript generates different code based on what was imported. Maybe inlining enums into literals? And if it doesn't now, it can start doing that in the future. |
OH, that's actually the perfect edge case! I was trying to think of an edge-case where checking imports would be necessary for just compiled JS code (i.e. not declarations) and couldn't come up with one. But I did suspect that there were other edge-cases. In the RCA I compared it to Babel:
And, of course, TS does have a check for this though, So what I can do is Could make that a separate PR though, as checking |
- ezolenko gave a good example of enums, which can indeed cause the compiled JS code to change based on imports
- so also check `!isolatedModules` as well
- I thought it might be the case that the code wouldn't handle other edge cases, but couldn't think of one off the top of my head
- ironically, I compared it to Babel, which transpiles per file, and Babel _requires_ `isolatedModules` to work
|
added another commit to handle think this is good to go now! |
agilgur5
changed the title
codeCache in most cases when imports change
Summary
The
codeCacheshould be invalidated when imports change if declarations are needed, because imports' types can change the resulting declaration.clean: true) #292EDIT: also added
!isolatedModulesto the check, see below comments regarding, e.g. enums, that could cause compiled JS code to change as well when imports changeDetails
checkImportsflag was set totruefor type-checking, butfalsefor compilationfalsebecause the compiled JS shouldn't change if an import changesfalsefor compilation if declarations are not neededSee my root cause analysis in #292 (comment)