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

Always send configFileDiagEvent and instead set triggerFile for more consistency #58462

Merged
merged 4 commits into from
May 7, 2024

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented May 7, 2024

We have following rules on when to send the configFileDiagEvent

  • When file is opened, we want to send the event for all the config files that get loaded for the first time
  • If the project was already loaded, we don't want to send the same configFileDiagEvent
  • But then if the file will be added to inferred project, we want to send configFileDiagEvent event for all loaded/looked up projects so that we can figure out if there are errors in config file which is the reason why the file went into inferred project
  • When there is change in config file, we always want to send configFileDiagEvent
  • We also want to send configFileDiagEvent if the diagnostics changed any time program update happens
    So this handles that ensuring we arent sending duplicate event

468d91c deletes the unnecessary comment about loading/reloading configured project
f738ccd actual change

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 7, 2024
@sheetalkamat sheetalkamat changed the title Always send configFileDiag and instead set triggerFile for more consistency Always send configFileDiagEvent and instead set triggerFile for more consistency May 7, 2024
const project = this.createAndLoadConfiguredProject(configFileName, reason);
project.skipConfigDiagEvent = true;
project.updateGraph();
private createLoadAndUpdateConfiguredProject(configFileName: NormalizedPath, reason: string, triggerFile: NormalizedPath | undefined) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed all the createConfiguredProjectAnd** methods except this one as now createConfiguredProject takes reason and sets the pending level to Full to correctly represent the state.

}
]
}
}
Info seq [hh:mm:ss:mss] event:
Copy link
Member Author

Choose a reason for hiding this comment

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

This is example of duplicate configFileDiagEvent

@jakebailey
Copy link
Member

Is there a linked bug that this fixes?

@sheetalkamat
Copy link
Member Author

This came up while i was working on finding default project #57196 and is followup to #58120

@sheetalkamat sheetalkamat merged commit b9c71c3 into main May 7, 2024
28 checks passed
@sheetalkamat sheetalkamat deleted the configDiag branch May 7, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants