Skip to content

Report errors correctly by invalidating super set of affected files #25534

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

Closed
wants to merge 1 commit into from

Conversation

sheetalkamat
Copy link
Member

Fixes #24986
The issue reported was with the graph a -> b -> c
When change was made c, the declaration file for b doesnt change (it still remains import {C}from "./c" export class B{ c: C}). From emit persepctive file a.ts doesnt need to be emitted (since its not part of affected files by that change) and without deleting errors for a.ts we wont see new error.
With this change, whenever change in declaration file for c happens, we not only delete errors for b.ts, we will delete errors for a.ts (since it references b.ts) and any file that will reference a.ts as well. This will hamper the performance in scenarios where errors didnt change (eg if a.ts never referenced b.c.d property) but finding out such dependencies would be costlier than re-generating errors.

@sheetalkamat sheetalkamat force-pushed the changeInDeepImportedFile branch from 82529f7 to edd0007 Compare July 10, 2018 17:53
@mhegazy
Copy link
Contributor

mhegazy commented Jul 10, 2018

Do we need to do anything here? is there a better way to detect cases where these transitive dependencies lead to invalidation? seems like most of the time transitive dependencies do not introduce errors. seems like this is a drastic change..

@sheetalkamat
Copy link
Member Author

Closing in favor of #25593

@sheetalkamat sheetalkamat deleted the changeInDeepImportedFile branch July 12, 2018 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants