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

Handle config file change and default project management #58486

Merged
merged 2 commits into from May 10, 2024
Merged

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented May 9, 2024

This is everything from #57196 except there is no change in functionality for finding default project. So that we can get this for 5.5 but behavior change for 5.6

This change includes:

  • Unless we have detected change in config file or we are reloading, once file is opened, its default config file name is never recalculated
  • All the paths to find default project - opening a file, reloading a project, just searching what's default project for a file or change in config file, use singular path for finding default project
  • When config file change is detected, we recalculate what is the default config name for open file and schedule update for project only if its new. If it exists, change in config should not cause the update to project just because file is open. The update happens to project only if it will be referencing config file or is changed for own config file
  • tryFindDefaultConfiguredProjectAndLoadAncestorsForOpenScriptInfo finds, creates, or reloads the default project and ancestor tree (for find all project)
  • tryFindDefaultConfiguredProjectForOpenScriptInfo finds, creates or reloads default project for the open file
  • When finding orphan configured projects, instead of keeping ref and its login on configured project, we go through open files and all the default projects and projects seen for finding default project are retained. This ensures whatever situation, the "should the project be kept alive" answer never changes because of some updates missing or something like that.

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels May 9, 2024
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@@ -663,6 +663,24 @@ Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Close:: WatchInfo: /us
Info seq [hh:mm:ss:mss] DirectoryWatcher:: Close:: WatchInfo: /user/username/projects/node_modules/@types 1 undefined Project: /user/username/projects/myproject/folder/tsconfig.json WatchType: Type roots
Info seq [hh:mm:ss:mss] Elapsed:: *ms DirectoryWatcher:: Close:: WatchInfo: /user/username/projects/node_modules/@types 1 undefined Project: /user/username/projects/myproject/folder/tsconfig.json WatchType: Type roots
Info seq [hh:mm:ss:mss] `remove Project::
Info seq [hh:mm:ss:mss] Project '/user/username/projects/myproject/tsconfig.json' (Configured)
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 exactly what happens when you open commonFile2 when it has tsconfig next to it that doesn't contain that file. This will be fixed to look for this project in #57196 but this makes the behavior consistent and not confuse when file belongs to right project or not .

@sheetalkamat sheetalkamat force-pushed the refactor branch 5 times, most recently from 3f75f76 to 3037e55 Compare May 9, 2024 18:32
@@ -306,22 +306,21 @@ ScriptInfos::
Info seq [hh:mm:ss:mss] FileWatcher:: Triggered with /a/b/projects/project/tsconfig.json 0:: WatchInfo: /a/b/projects/project/tsconfig.json 2000 undefined Project: /a/b/projects/project/tsconfig.json WatchType: Config file
Info seq [hh:mm:ss:mss] Scheduled: /a/b/projects/project/tsconfig.json
Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: /a/b/projects/project/src/index.ts ProjectRootPath: /a/b/projects/proj:: Result: /a/b/projects/project/tsconfig.json
Info seq [hh:mm:ss:mss] Scheduled: /a/b/projects/project/tsconfig.json, Cancelled earlier one
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 doesnt reschedule update for project if its already scheduled through set of updated projects

@@ -495,20 +495,21 @@ ScriptInfos::

Info seq [hh:mm:ss:mss] FileWatcher:: Triggered with /a/b/tsconfig.json 1:: WatchInfo: /a/b/tsconfig.json 2000 undefined Project: /a/b/tsconfig.json WatchType: Config file
Info seq [hh:mm:ss:mss] Scheduled: /a/b/tsconfig.json
Info seq [hh:mm:ss:mss] getConfigFileNameForFile:: File: /a/b/src/file1.ts ProjectRootPath: undefined:: Result: /a/b/tsconfig.json
Copy link
Member Author

Choose a reason for hiding this comment

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

Earlier we use to skip if file was not part of inferred project, but change in configFile can have impact on whats the default project, so we upate the default config File Name for this file in cache and make sure that the project is present. Only if its new we schedule update otherwise the project doesnt need to be updated.

@sheetalkamat sheetalkamat changed the title [wip] Handle config file change and default project management Handle config file change and default project management May 9, 2024
@sheetalkamat sheetalkamat merged commit fadc83b into main May 10, 2024
28 checks passed
@sheetalkamat sheetalkamat deleted the refactor branch May 10, 2024 21:50
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.

None yet

3 participants