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

Fix virtual file invalidation #12591

Merged
merged 2 commits into from
Oct 19, 2018

Conversation

filipesilva
Copy link
Contributor

@filipesilva filipesilva commented Oct 15, 2018

Followup to #12588, should fix #12260 going forward.

Fix #9669
Fix #12657

@filipesilva filipesilva added needs: discussion On the agenda for team meeting to determine next steps target: major This PR is targeted for the next major release labels Oct 15, 2018
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done 👍

@filipesilva filipesilva added state: blocked target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release needs: discussion On the agenda for team meeting to determine next steps labels Oct 15, 2018
@@ -80,8 +87,13 @@ export class WebpackCompilerHost implements ts.CompilerHost {
invalidate(fileName: string): void {
const fullPath = this.resolve(fileName);

if (this._memoryHost.exists(fullPath)) {
this._memoryHost.delete(fullPath);
if (fullPath.endsWith('.ts')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to also check if it no longer exists on disk as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, and I think that's what (rightfully) causing that failure on test-large.

this._sourceFileCache.delete(fullPath);

try {
const exists = this._syncHost.isFile(fullPath);
if (exists) {
this._changedFiles.add(fullPath);
}
} catch { }
} catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isFile may or may not throw if the file exists. Some host do and some do not.
Maybe something similar to the following:

let exists = false;
try {
  exists = this._syncHost.isFile(fullPath);
  if (exists) {
    this._changedFiles.add(fullPath);
  }
} catch { }

// File doesn't exist anymore, so we should delete the related virtual files.
if (!exists && fullPath.endsWith('.ts')) {
  this._virtualFileExtensions.forEach(ext => {
    const virtualFile = (fullPath.slice(0, -3) + ext) as Path;
    if (this._memoryHost.exists(virtualFile)) {
      this._memoryHost.delete(virtualFile);
    }
  });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@alan-agius4
Copy link
Collaborator

@filipesilva I think this also solves #12620?

@filipesilva
Copy link
Contributor Author

I'm not sure it's related. Have you tested it?

@alan-agius4
Copy link
Collaborator

@filipesilva I gave it a shot and cannot replicate. Will ask for repo.

@alexeagle alexeagle merged commit a6ffa2b into angular:master Oct 19, 2018
@filipesilva filipesilva deleted the fix-invalidate-context branch October 26, 2018 09:47
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
5 participants