Skip to content

Fix incorrect setting of nonRecursive watch field#53675

Merged
sheetalkamat merged 1 commit into
mainfrom
incorrectField
Apr 5, 2023
Merged

Fix incorrect setting of nonRecursive watch field#53675
sheetalkamat merged 1 commit into
mainfrom
incorrectField

Conversation

@sheetalkamat
Copy link
Copy Markdown
Member

It was meant to be nonRecursive and we treat as same but set was set incorrectly from start when added in #25811

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 5, 2023
Copy link
Copy Markdown
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

I can't say I know what's going on here but there's clearly a comment right above there that says it should be non-recursive 😄

Comment on lines 139 to 140
| //vda1cs4850/dir/somefile.d.ts | | |
| //vda1cs4850/dir/subdir/somefile.d.ts | | |
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.

What does it mean when Recursive is blank?

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.

If second column (path to watch) is blank recursive is blank meaning these kind of paths will be ignored for setting directory watchers

@DanielRosenwasser
Copy link
Copy Markdown
Member

So there's a comment above like

// Instead of watching root, watch directory in root to avoid watching excluded directories not needed for module resolution

How does that interact with stuff like node_modules which is typically ignored, but which typically is included into the program anyway?

@sheetalkamat
Copy link
Copy Markdown
Member Author

@sheetalkamat
Copy link
Copy Markdown
Member Author

So there's a comment above like

// Instead of watching root, watch directory in root to avoid watching excluded directories not needed for module resolution

How does that interact with stuff like node_modules which is typically ignored, but which typically is included into the program anyway?

say a.ts has import to bar. The failed lookup location will be root/node_modules/bar/bar.d.ts
At this point we want to watch root/node_modules instead of watching root. This helps with scenarios where there are unrelated folders in root. Eg say you have root/scripts. If you were watching root, you would need to go through all the the the resolutions and their failed lookups only to find that it doesnt matter. So lot of unnecessary work.

@sheetalkamat sheetalkamat merged commit d07b1b9 into main Apr 5, 2023
@sheetalkamat sheetalkamat deleted the incorrectField branch April 5, 2023 21:17
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants