FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ TypeScript Public

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

Merged
Sheetal Nandi (sheetalkamat) merged 8 commits into
masterfrom
watchOptions
Dec 11, 2019
ConversationCommits (8)ChecksFiles changed
Merged

Add watchOptions to tsconfig and allow supplying them on command line as well#35615
Sheetal Nandi (sheetalkamat) merged 8 commits into
masterfrom
watchOptions

Conversation

Sheetal Nandi (sheetalkamat) commented Dec 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

TypeScript Bot (typescript-bot) commented Dec 10, 2019

Copy link
Copy Markdown
Contributor

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 Sheetal Nandi (@sheetalkamat), Andrew Casey (@amcasey), Matt Bierner (@mjbvz), Mine Starks (@minestarks) for you. Feel free to loop in other consumers/maintainers if necessary

Sheetal Nandi (sheetalkamat) commented Dec 10, 2019

Copy link
Copy Markdown
Member Author

TypeScript Bot (@typescript-bot) pack this

TypeScript Bot (typescript-bot) commented Dec 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Heya Sheetal Nandi (@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 (typescript-bot) commented Dec 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hey Sheetal Nandi (@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.

This was referenced Dec 10, 2019
Closed
Closed
Closed
Closed
Sheetal Nandi (sheetalkamat) deleted the watchOptions branch December 11, 2019 21:26

Aranha (aranhaagency) commented Dec 13, 2019

Copy link
Copy Markdown

Nice work!

Andrei Alecu (andreialecu) mentioned this pull request Dec 31, 2019
Closed

Andrei Alecu (andreialecu) commented Dec 31, 2019

Copy link
Copy Markdown

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

Daniel Rosenwasser (DanielRosenwasser) reviewed Jan 7, 2020
View reviewed changes
Comment thread src/testRunner/unittests/tscWatch/consoleClearing.ts
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",

Daniel Rosenwasser (DanielRosenwasser) Jan 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What does undefined refer to here?

Sheetal Nandi (sheetalkamat) Jan 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

undefined is watchOptions passed to the file watcher

Daniel Rosenwasser (DanielRosenwasser) reviewed Jan 7, 2020
View reviewed changes
Comment thread 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,

Daniel Rosenwasser (DanielRosenwasser) Jan 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Daniel Rosenwasser (DanielRosenwasser) Jan 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Also, what's the default?

Sheetal Nandi (sheetalkamat) Jan 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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..)

Daniel Rosenwasser (DanielRosenwasser) Jan 7, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Sheetal Nandi (sheetalkamat) Jan 8, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Daniel Rosenwasser (DanielRosenwasser) reviewed Jan 7, 2020
View reviewed changes
Comment thread 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",

Daniel Rosenwasser (DanielRosenwasser) Jan 7, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Is this polling for files, directories, or both?

Sheetal Nandi (sheetalkamat) Jan 7, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

This was referenced Jan 24, 2020
Closed
Closed
This was referenced May 21, 2020
Closed
Closed
mntnoe mentioned this pull request Jun 8, 2020
Closed
github-actions Bot mentioned this pull request Aug 14, 2022
Merged
Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Daniel Rosenwasser (DanielRosenwasser) DanielRosenwasser left review comments
Ryan Cavanaugh (RyanCavanaugh) RyanCavanaugh approved these changes
Wesley Wigham (weswigham) Awaiting requested review from weswigham
Orta Therox (orta) Awaiting requested review from orta
Nathan Shively-Sanders (sandersn) Awaiting requested review from sandersn
Andrew Casey (amcasey) Awaiting requested review from amcasey
Andrew Branch (andrewbranch) Awaiting requested review from andrewbranch
Eli Barzilay (elibarzilay) Awaiting requested review from elibarzilay
Ron Buckton (rbuckton) Awaiting requested review from rbuckton

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

100% CPU when installing node_modules while a file is open

6 participants

Back | FazBrowse Home | New Git URL