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

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

Merged
merged 8 commits into from Dec 11, 2019

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Dec 10, 2019

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 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
Copy link
Member Author

sheetalkamat commented Dec 10, 2019

@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
8 checks passed
@sheetalkamat sheetalkamat deleted the watchOptions branch Dec 11, 2019
@aranhaagency
Copy link

aranhaagency commented Dec 13, 2019

Nice work!

@andreialecu
Copy link

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

@@ -2,8 +2,8 @@ namespace ts.tscWatch {
describe("unittests:: tsc-watch:: console clearing", () => {
const currentDirectoryLog = "Current directory: / CaseSensitiveFileNames: false\n";
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

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

What does undefined refer to here?

Copy link
Member Author

@sheetalkamat sheetalkamat Jan 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

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

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

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

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

Also, what's the default?

Copy link
Member Author

@sheetalkamat sheetalkamat Jan 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

@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

@sheetalkamat sheetalkamat Jan 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.

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

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

Is this polling for files, directories, or both?

Copy link
Member Author

@sheetalkamat sheetalkamat Jan 7, 2020

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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants