Skip to content

findAllReferences: Reduce node.getSourceFile() calls#23524

Merged
2 commits merged into
masterfrom
getTextSpanAndFileName
Apr 18, 2018
Merged

findAllReferences: Reduce node.getSourceFile() calls#23524
2 commits merged into
masterfrom
getTextSpanAndFileName

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Apr 18, 2018

We were recomputing the source file in a lot of places, this should ensure that we reuse the result.

@ghost ghost requested a review from sheetalkamat April 18, 2018 20:24
Comment thread src/services/findAllReferences.ts Outdated
const sourceFile = node.getSourceFile();
const textSpan = getTextSpan(isComputedPropertyName(node) ? node.expression : node, sourceFile);
return { containerKind: ScriptElementKind.unknown, containerName: "", fileName: sourceFile.fileName, kind, name, textSpan, displayParts };
return { containerKind: ScriptElementKind.unknown, containerName: "", kind, name, displayParts, ...getTextSpanAndFileName(isComputedPropertyName(node) ? node.expression : node) };
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Using this helper means we are creating unnecessary object and throwing it rightaway. Given that this is called in so many places, i would rather have it inlined than create this indirection.

@ghost ghost merged commit 8f1bdc7 into master Apr 18, 2018
@ghost ghost deleted the getTextSpanAndFileName branch April 18, 2018 22:24
@microsoft microsoft locked and limited conversation to collaborators Jul 30, 2018
This pull request was closed.
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.

1 participant