Skip to content

Fix incorrect setting of nonRecursive watch field #53675

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

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Conversation

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

@sheetalkamat
Copy link
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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