Skip to content

Remove cached resolvedModule result if the target file was deleted#11460

Merged
zhengbli merged 7 commits into
microsoft:masterfrom
zhengbli:11206
Oct 11, 2016
Merged

Remove cached resolvedModule result if the target file was deleted#11460
zhengbli merged 7 commits into
microsoft:masterfrom
zhengbli:11206

Conversation

@zhengbli
Copy link
Copy Markdown

@zhengbli zhengbli commented Oct 8, 2016

Currently we cache successfully resolved module result in SourceFile object, even when the target file is deleted. This prevents the module not found error being removed if the deleted file was added back.

Fixes #11206 .

@vladima
Copy link
Copy Markdown
Contributor

vladima commented Oct 10, 2016

this is not the right fix - let's discuss it offline

@zhengbli
Copy link
Copy Markdown
Author

zhengbli commented Oct 10, 2016

@vladima Done updating. I'll do a squash merge after your reiview. Thanks!

Comment thread src/server/lsHost.ts Outdated
resolveModuleNames(moduleNames: string[], containingFile: string): ResolvedModule[] {
return this.resolveNamesWithLocalCache(moduleNames, containingFile, this.resolvedModuleNames, this.resolveModuleName, m => m.resolvedModule);
return this.resolveNamesWithLocalCache(moduleNames, containingFile, this.resolvedModuleNames, this.resolveModuleName, m => {
if (!m.resolvedModule) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it will be better to do the following:

  • add a constraint R extends { resolvedFileName?: string } to R type parameter in resolveNamesWithLocalCache - both ResolvedModule and ResolvedTypeReferenceDirective satisfy this constraint.
  • inside moduleResolutionIsValid so something like:
// lastDeletedFileName should be declared as somewhere before the `for-of` loop 
// const lastDeletedFileName = this.project.projectService.lastDeletedFile && this.project.projectService.lastDeletedFile.fileName;

const result = getResult(resolution);
if (result) {
    if (result.resolvedFileName && result.resolvedFileName === lastDeletedFileName) {
        return false;
    }
    return true;
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Great suggestion! Done updating.

@zhengbli zhengbli merged commit 2671668 into microsoft:master Oct 11, 2016
@zhengbli zhengbli deleted the 11206 branch October 11, 2016 20:41
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File rename not handled correctly in 2.0.3

3 participants