Skip to content

Files watcher exclude - do not ignore changes to the node_modules folder itself.#125801

Merged
bpasero merged 1 commit into
microsoft:mainfrom
Aaaaash:fix/file-excludes-default-value
Jun 16, 2021
Merged

Files watcher exclude - do not ignore changes to the node_modules folder itself.#125801
bpasero merged 1 commit into
microsoft:mainfrom
Aaaaash:fix/file-excludes-default-value

Conversation

@Aaaaash

@Aaaaash Aaaaash commented Jun 9, 2021

Copy link
Copy Markdown
Contributor

This PR fixes #125886

Glob online tester

node_modules-watcger

@Aaaaash

Aaaaash commented Jun 9, 2021

Copy link
Copy Markdown
Contributor Author

same issue

@Aaaaash

Aaaaash commented Jun 9, 2021

Copy link
Copy Markdown
Contributor Author

Friendly ping @bpasero

@bpasero

bpasero commented Jun 9, 2021

Copy link
Copy Markdown
Contributor

What issue does this address?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jun 9, 2021
@Aaaaash

Aaaaash commented Jun 10, 2021

Copy link
Copy Markdown
Contributor Author

updated

@bpasero bpasero removed the info-needed Issue requires more information from poster label Jun 10, 2021
@bpasero bpasero added this to the Backlog milestone Jun 10, 2021
'files.watcherExclude': {
'type': 'object',
'default': isWindows /* https://github.com/microsoft/vscode/issues/23954 */ ? { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/*/**': true, '**/.hg/store/**': true } : { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/**': true, '**/.hg/store/**': true },
'default': isWindows /* https://github.com/microsoft/vscode/issues/23954 */ ? { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/*/**': true, '**/.hg/store/**': true } : { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/**/*': true, '**/.hg/store/**': true },

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Aaaaash any reason we wouldn't just pick the exact same pattern as on Windows? From my testing, the advantage of '**/node_modules/*/**' over '**/node_modules/**/*' seems to be that you would see changes to top level folders in the node_modules folder, but nothing deeper. So essentially the explorer refreshes nicely on operations such as yarn add <module>.

With your suggested pattern, I only see node_modules folder appearing and hiding, but not changes inside the folder.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ping @Aaaaash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late reply, you are right, so we can use the same pattern on Windows and Linux/macOS ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I think so, can you update the PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

@Aaaaash Aaaaash force-pushed the fix/file-excludes-default-value branch from d8e7cc0 to 814c156 Compare June 16, 2021 05:57
@bpasero bpasero modified the milestones: Backlog, June 2021 Jun 16, 2021
@bpasero

bpasero commented Jun 16, 2021

Copy link
Copy Markdown
Contributor

Thanks 👍

@bpasero bpasero merged commit 237a750 into microsoft:main Jun 16, 2021
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 31, 2021
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.

macOS: Changes to node_modules folder are ignored

2 participants