NotificationsYou must be signed in to change notification settings
Fork13.2k
Star107k

Add watchOptions to tsconfig and allow supplying them on command line as well#35615

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

sheetalkamat merged 8 commits intomasterfromwatchOptions

Dec 11, 2019

Merged

Add watchOptions to tsconfig and allow supplying them on command line as well#35615

sheetalkamat merged 8 commits intomasterfromwatchOptions

Dec 11, 2019

Conversation

Copy link

Member

sheetalkamat commentedDec 10, 2019•
edited
Loading

This addswatchOptions as entry parallel tocompilerOptions in tsconfig to specifywatch specific options. Eg.watchFile strategy etc. It would also allow us to set ignores for watches or some such options in future, or allowing delayed project watches in the server and others.
Currently watchOptions haswatchFile strategy to watch files,watchDirectory strategy to watch individual directory on the OS that don't support native recursive watching andfallbackPolling method to poll when fs related watching fails.
This also adds the delayed update to child watches when the recursive directory watches are invoked. It can be disabled by settingsynchronousWatchDirectory inwatchOptions
Fixes100% CPU when installing node_modules while a file is open #30208,VSCode and intellisense are broken and unstable #32033,Code Helper high CPU (electron_node tsserver.js) #34401

sheetalkamat added7 commits

December 10, 2019 11:37

Create different watch options in compiler options

f69e166

Thread through the new watch options

d5aa608

Actually use the options passed through for watch strategy

06d21a3

Support delay on updating child directory watches

d26fd7d

Make watchOptions separate from compilerOptions

909f458

Support passing watch options from command line

985a9b6

Handle displaying of watchOptions

11d7f4f

Copy link

Collaborator

typescript-bot commentedDec 10, 2019

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping@sheetalkamat,@amcasey,@mjbvz,@minestarks for you. Feel free to loop in other consumers/maintainers if necessary

sheetalkamat requested review fromRyanCavanaugh,amcasey,andrewbranch,elibarzilay,orta,rbuckton,sandersn andweswigham

December 10, 2019 21:04

Copy link

MemberAuthor

sheetalkamat commentedDec 10, 2019

@typescript-bot pack this

Copy link

Collaborator

typescript-bot commentedDec 10, 2019•
edited
Loading

Heya@sheetalkamat, I've started to run the tarball bundle task on this PR at11d7f4f. You can monitor the buildhere. It should now contribute to this PR's status checks.

Copy link

Collaborator

typescript-bot commentedDec 10, 2019•
edited
Loading

Hey@sheetalkamat, I've packed this intoan installable tgz. You can install it for testing by referencing it in yourpackage.json like so:

{    "devDependencies": {        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/57410/artifacts?artifactName=tgz&fileId=C56D34170AE2E201F9C6006F088E430E6DCD20FE710334CDAA4E0AA14B6BCD2A02&fileName=/typescript-3.8.0-insiders.20191210.tgz"    }}

and then runningnpm install.

There is also a playgroundfor this build.

This was referencedDec 10, 2019

100% CPU when installing node_modules while a file is open#30208

Closed

VSCode and intellisense are broken and unstable#32033

Closed

Code Helper high CPU (electron_node tsserver.js)#34401

Closed

tsserver is acting strangely on VS Code Remote-WSL (WSL2)#34905

Closed

RyanCavanaugh approved these changes

Dec 11, 2019

View reviewed changes

Merge branch 'master' into watchOptions

600bc04

sheetalkamat merged commit236012e intomaster

Dec 11, 2019

sheetalkamat deleted the watchOptions branch

December 11, 2019 21:26

Copy link

aranhaagency commentedDec 13, 2019

Nice work!

andreialecu mentioned this pull request

Dec 31, 2019

Initializing js/ts language features extremely slow on WSL2microsoft/vscode#87921

Closed

Copy link

andreialecu commentedDec 31, 2019

Should this help with#33335 ? If so, are there any aditional steps necessary? I've installed the latest nightly and selected it in VS Code but it doesn't seem to have helped.

Info 0    [12:52:10.97] Starting TS ServerInfo 1    [12:52:10.98] Version: 3.8.0-dev.20191231

DanielRosenwasser reviewed

Jan 7, 2020

View reviewed changes

src/testRunner/unittests/tscWatch/consoleClearing.ts

		constfileWatcherAddedLog=[
		"FileWatcher:: Added:: WatchInfo: /f.ts 250 Source file\n",
		"FileWatcher:: Added:: WatchInfo: /a/lib/lib.d.ts 250 Source file\n"
		"FileWatcher:: Added:: WatchInfo: /f.ts 250undefinedSource file\n",

Copy link

Member

DanielRosenwasserJan 7, 2020

Choose a reason for hiding this comment

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

What doesundefined refer to here?

Copy link

MemberAuthor

sheetalkamatJan 7, 2020

Choose a reason for hiding this comment

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

undefined is watchOptions passed to the file watcher

DanielRosenwasser reviewed

Jan 7, 2020

View reviewed changes

src/compiler/commandLineParser.ts

		name:"synchronousWatchDirectory",
		type:"boolean",
		category:Diagnostics.Advanced_Options,
		description:Diagnostics.Synchronously_call_callbacks_and_update_the_state_of_directory_watchers_on_platforms_that_don_t_support_recursive_watching_natively,

Copy link

Member

DanielRosenwasserJan 7, 2020

Choose a reason for hiding this comment

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

As a user, is this relevant to me? What do I need to know? It's not clear to me what this option does.

Copy link

Member

DanielRosenwasserJan 7, 2020

Choose a reason for hiding this comment

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

Also, what's the default?

Copy link

MemberAuthor

sheetalkamatJan 7, 2020

Choose a reason for hiding this comment

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

default is false and setting this to true would get you behaviour in 3.7.. This is new flag that when not set, on recursive directory watching doesnt update the watcher status right away but instead sets a timeout to update the watches for recursive structure.. (eg. when say removing or creating node_modules folder, while npm install is going, the recursive watchers aren't updated right away but on timeout so we aren't eg adding watches on directories that will anyways be deleted or recreated..)

Copy link

Member

DanielRosenwasserJan 7, 2020•
edited
Loading

Choose a reason for hiding this comment

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

Who would you say would use this feature? Here's how I'm describing it.

Disable deferred watching on directories. Deferred watching is useful when lots of file changes might occur at once (e.g. a change innode_modules from runningnpm install), but you might want to disable it with this flag for some atypical setups.

Copy link

MemberAuthor

sheetalkamatJan 8, 2020

Choose a reason for hiding this comment

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

This is kind of backup flag that can disable this behavior if changes aren't reflected correctly/sooner.

DanielRosenwasser reviewed

Jan 7, 2020

View reviewed changes

src/compiler/commandLineParser.ts

		description:Diagnostics.Specify_strategy_for_watching_directory_on_platforms_that_don_t_support_recursive_watching_natively_Colon_UseFsEvents_default_FixedPollingInterval_DynamicPriorityPolling,
		},
		{
		name:"fallbackPolling",