Watch extended configs if present #41493

molisani · 2020-11-11T17:49:56Z

Inspects extendedSourceFiles from configFile in CompilerOptions and starts a watcher for each file. Watch callback invokes scheduleProgramReload just like the watcher for the main config file. Watcher pattern modelled after watcher map for missing files. Also provides shared watcher map for situations where multiple projects are open at once (tsbuild/tsserver).

Added new test cases, including chain of extended configs and ones copied from watchEnvironment/watchOptions with extended configs to verify they are being watched.

src/compiler/watchPublic.ts

src/compiler/watchUtilities.ts

src/testRunner/unittests/tscWatch/watchEnvironment.ts

src/compiler/watchPublic.ts

src/server/editorServices.ts

src/server/project.ts

src/testRunner/unittests/tsserver/configuredProjects.ts

src/server/editorServices.ts

molisani · 2020-11-19T23:34:19Z

I've rebased onto master, but I'm now getting some (seemingly) unrelated errors with some jsx baselines. Here are my local copies of the two failing baseline tests: jsFileCompilationTypeArgumentSyntaxOfCall.errors.txt and jsxCheckJsxNoTypeArgumentsAllowed.errors.txt. I'm going to do some more investigating, but wanted to report my local copies here on the PR if anyone could advise in this situation. This appears to be a known issue that is being fixed by #41602

src/compiler/watchUtilities.ts

sheetalkamat · 2020-12-02T02:35:22Z

src/compiler/watchUtilities.ts

+    ) {
+        const extendedSourceFiles = configFile.extendedSourceFiles || emptyArray;
+        // TODO(rbuckton): Should be a `Set` but that requires changing the below code that uses `mutateMap`
+        const newExtendedConfigFilesMap = arrayToMap(extendedSourceFiles, identity, returnTrue);


Key needs to be Path, value needs to be file name to watch

This doesnot create the key with Path

Ah, I missed the meaning of this the first time around. I'll pass something to allow converting from string to Path.

src/server/editorServices.ts

molisani

Will update to correctly account for removals, and add new tests. Had a few questions about latest review.

src/compiler/watchUtilities.ts

molisani · 2020-12-04T22:57:05Z

While creating a test case for removing projects, I noticed that I had not considered the builder's watch logic. I'll replicate the "shared" watcher pattern from the server here as well.

sheetalkamat

I have some more feedback but i am going to try fixing some of those myself as that might be easier

src/compiler/tsbuildPublic.ts

src/compiler/watchPublic.ts

sheetalkamat · 2020-12-10T21:03:34Z

src/compiler/watchUtilities.ts

+    ) {
+        const extendedSourceFiles = configFile.extendedSourceFiles || emptyArray;
+        // TODO(rbuckton): Should be a `Set` but that requires changing the below code that uses `mutateMap`
+        const newExtendedConfigFilesMap = arrayToMap(extendedSourceFiles, identity, returnTrue);


This doesnot create the key with Path

molisani · 2020-12-10T21:45:53Z

I have some more feedback but i am going to try fixing some of those myself as that might be easier

Thank you very much for those suggestions. I merged all of them and then applied the toPath conversion.

sheetalkamat · 2020-12-11T00:20:41Z

Please cherry pick e2d178f as i dont have permission to update your fork.

Added new `WatchType` for extended config files. Refactored watch map update to separate function, relocated call sites. Removed unnecessary test cases and relocated with new tests in programUpdates.

Update `updateExtendedConfigFilesWatch` to read from a `TsConfigSourceFile` to get `extendedSourceFiles`. Add watcher map to `ConfiguredProject` in the server. New test cases to verify correct events triggered and extended files are being watched properly.

Removes unnecessary actions in extended config watcher callback function. Updates tests to match.

New shared watcher map in ProjectService that stores callbacks per project to be invoked when the file watcher is triggered. The FileWatcher is created with the watch options of the first Project to watch the extended config.

Remove all server-related utility functions/types from watchUtilities. Store config-project mapping and config file watchers inside ProjectService with new private methods to add or remove projects.

Creates SharedExtendedConfigFileWatcher in both editorServices (tsserver) and tsbuildPublic. The file watcher is responsible for triggering a full project reload for the contained projects. Upon reload, any configs that are no longer related to a project have their watchers updated to match. New test cases to confirm that the file watchers for extended configs are closed when the project is closed.

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>

molisani · 2020-12-11T00:50:06Z

Please cherry pick e2d178f as i dont have permission to update your fork.

The unified SharedExtendedConfigFileWatcher makes a lot more sense now that it's being used by tsbuild and tsserver. I also had to rebase due to a merge conflict in src/server/project.ts but it was a small one. Thanks again for your help.

typescript-bot assigned sheetalkamat Nov 11, 2020

typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Nov 11, 2020

molisani force-pushed the watch-tsconfig-extends branch from bdf7c2e to ad2fbaa Compare November 11, 2020 17:59

sheetalkamat requested changes Nov 11, 2020

View reviewed changes

molisani force-pushed the watch-tsconfig-extends branch from 053e9b4 to 31f0d50 Compare November 13, 2020 03:26

molisani commented Nov 13, 2020

View reviewed changes

src/server/editorServices.ts Show resolved Hide resolved

src/server/editorServices.ts Outdated Show resolved Hide resolved

molisani force-pushed the watch-tsconfig-extends branch from 31f0d50 to 4d59cc6 Compare November 13, 2020 04:06

molisani requested a review from sheetalkamat November 13, 2020 04:39

sheetalkamat requested changes Nov 13, 2020

View reviewed changes

molisani force-pushed the watch-tsconfig-extends branch from 588bee3 to 36c29cb Compare November 14, 2020 06:31

molisani requested a review from sheetalkamat November 16, 2020 22:31

sheetalkamat requested changes Nov 17, 2020

View reviewed changes

JoostK mentioned this pull request Nov 18, 2020

feat(language-service): watch extended configs for option changes angular/angular#39741

Closed

molisani force-pushed the watch-tsconfig-extends branch from d091cb9 to 4224dd7 Compare November 19, 2020 22:42

molisani force-pushed the watch-tsconfig-extends branch 2 times, most recently from 67dcc8e to 86905be Compare November 20, 2020 03:38

molisani requested a review from sheetalkamat November 20, 2020 04:17

sandersn added this to Not started in PR Backlog Nov 23, 2020

sandersn moved this from Not started to Needs review in PR Backlog Nov 23, 2020

sheetalkamat reviewed Dec 2, 2020

View reviewed changes

src/compiler/watchUtilities.ts Outdated Show resolved Hide resolved

sheetalkamat requested changes Dec 2, 2020

View reviewed changes

PR Backlog automation moved this from Needs review to Waiting on author Dec 2, 2020

molisani commented Dec 2, 2020

View reviewed changes

src/compiler/watchUtilities.ts Outdated Show resolved Hide resolved

molisani force-pushed the watch-tsconfig-extends branch 2 times, most recently from 1acd39c to 2284a6c Compare December 9, 2020 22:49

molisani requested a review from sheetalkamat December 9, 2020 23:02

sheetalkamat requested changes Dec 10, 2020

View reviewed changes

molisani and others added 10 commits December 10, 2020 19:46

Watch extended configs if present

c0b6fb5

Address code review comments

55d4e3e

Added new `WatchType` for extended config files. Refactored watch map update to separate function, relocated call sites. Removed unnecessary test cases and relocated with new tests in programUpdates.

Simplify watcher callback, fix tests

fa918ed

Removes unnecessary actions in extended config watcher callback function. Updates tests to match.

Share extended config watchers across projects in server

1a527d6

New shared watcher map in ProjectService that stores callbacks per project to be invoked when the file watcher is triggered. The FileWatcher is created with the watch options of the first Project to watch the extended config.

Refactor shared extended config map and watchers

963ccf9

Remove all server-related utility functions/types from watchUtilities. Store config-project mapping and config file watchers inside ProjectService with new private methods to add or remove projects.

Apply suggestions from code review

e7ff4b8

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>

Map extended config files by path

45f19cc

Move shared watcher into utilities and add more tests

7d25e43

sheetalkamat approved these changes Dec 11, 2020

View reviewed changes

PR Backlog automation moved this from Waiting on author to Needs merge Dec 11, 2020

molisani force-pushed the watch-tsconfig-extends branch from 3a0c0dd to 7d25e43 Compare December 11, 2020 00:49

sheetalkamat merged commit 716b167 into microsoft:master Dec 11, 2020

PR Backlog automation moved this from Needs merge to Done Dec 11, 2020

molisani deleted the watch-tsconfig-extends branch December 11, 2020 15:31

MartinJohns mentioned this pull request Oct 30, 2022

TS server not reload tsconfig when extended tsconfig is installed #51351

Open

Watch extended configs if present #41493

Watch extended configs if present #41493

molisani commented Nov 11, 2020 •

edited

molisani commented Nov 19, 2020 •

edited

sheetalkamat Dec 2, 2020

sheetalkamat Dec 10, 2020

molisani Dec 10, 2020

molisani left a comment

molisani commented Dec 4, 2020

sheetalkamat left a comment

sheetalkamat Dec 10, 2020

molisani commented Dec 10, 2020

sheetalkamat commented Dec 11, 2020

molisani commented Dec 11, 2020

Watch extended configs if present #41493

Watch extended configs if present #41493

Conversation

molisani commented Nov 11, 2020 • edited

molisani commented Nov 19, 2020 • edited

sheetalkamat Dec 2, 2020

Choose a reason for hiding this comment

sheetalkamat Dec 10, 2020

Choose a reason for hiding this comment

molisani Dec 10, 2020

Choose a reason for hiding this comment

molisani left a comment

Choose a reason for hiding this comment

molisani commented Dec 4, 2020

sheetalkamat left a comment

Choose a reason for hiding this comment

sheetalkamat Dec 10, 2020

Choose a reason for hiding this comment

molisani commented Dec 10, 2020

sheetalkamat commented Dec 11, 2020

molisani commented Dec 11, 2020

molisani commented Nov 11, 2020 •

edited

molisani commented Nov 19, 2020 •

edited