Skip to content

Conversation

sheetalkamat
Copy link
Member

With this updateGraph is used only in locations we are certain project is dirty
This reduces the number of times updateGraph is called when there is no change

With this updateGraph is used only in locations we are certain project is dirty
@@ -337,7 +337,8 @@ namespace ts.server {
return `Project: ${project ? project.getProjectName() : ""} WatchType: ${watchType}`;
}

function updateProjectIfDirty(project: Project) {
/*@internal*/
export function updateProjectIfDirty(project: Project) {
return project.dirty && project.updateGraph();
Copy link
Member

Choose a reason for hiding this comment

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

Why not put this check directly in updateGraph? Are there cases where we want to force an update regardless?

Copy link
Member Author

Choose a reason for hiding this comment

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

There are cases we already know that project is dirty..
Updategraph is also overridden to do extra things depending on project type so update graph is not place where you want to do this check

Copy link
Member

Choose a reason for hiding this comment

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

As a caller, how do I know whether I should call updateProjectIfDirty or updateGraph?

@sheetalkamat
Copy link
Member Author

If you know project is dirty for sure you use updateGraph otherwise updateIfDirty. (eg. when you are updating the root file names because new files were added to the project, changing compiler options, creating project first time is when you use updateGraph, rest all is updateIfDirty)

@sheetalkamat sheetalkamat merged commit 9df8831 into master Aug 2, 2018
@sheetalkamat sheetalkamat deleted the updateGraphIfDirty branch August 2, 2018 23:32
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.

3 participants