KEMBAR78
Add watchOptions to tsconfig and allow supplying them on command line as well by sheetalkamat · Pull Request #35615 · microsoft/TypeScript · GitHub
Skip to content

Conversation

@sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Dec 10, 2019

@typescript-bot
Copy link
Collaborator

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
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 10, 2019

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 10, 2019

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.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 running npm install.


There is also a playground for this build.

@sheetalkamat sheetalkamat merged commit 236012e into master Dec 11, 2019
@sheetalkamat sheetalkamat deleted the watchOptions branch December 11, 2019 21:26
@aranhaagency
Copy link

Nice work!

@andreialecu
Copy link

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 Server
Info 1    [12:52:10.98] Version: 3.8.0-dev.20191231

const fileWatcherAddedLog = [
"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 250 undefined Source file\n",
Copy link
Member

Choose a reason for hiding this comment

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

What does undefined refer to here?

Copy link
Member Author

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

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

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

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
Member Author

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

@DanielRosenwasser DanielRosenwasser Jan 7, 2020

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 in node_modules from running npm install), but you might want to disable it with this flag for some atypical setups.

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 kind of backup flag that can disable this behavior if changes aren't reflected correctly/sooner.

description: Diagnostics.Specify_strategy_for_watching_directory_on_platforms_that_don_t_support_recursive_watching_natively_Colon_UseFsEvents_default_FixedPollingInterval_DynamicPriorityPolling,
},
{
name: "fallbackPolling",
Copy link
Member

Choose a reason for hiding this comment

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

Is this polling for files, directories, or both?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its for both.. Any fallback when filesystem native fails uses this method

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

100% CPU when installing node_modules while a file is open

6 participants