microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.42k stars 28.62k forks source link

macOS: Changes to node_modules folder are ignored #125886

Closed Aaaaash closed 3 years ago

Aaaaash commented 3 years ago

Does this issue occur when all extensions are disabled?: Yes

Steps to Reproduce:

  1. Open Node.js project
  2. Execute npm install in terminal
  3. node_modules is not shown in Explorer side bar.

node

It seems to be by designed for performance reasons #23954

Actually there is a difference between Windows and macOS/Linux: we ignore /node_modules/ but that should actually not ignore changes to the node_modules folder itself. The real issue here is that our glob library is matching node_modules for a pattern of /node_modules/ when it should not.

For now, files.watcherExclude has different default value for Windows and macOS/Linux.

Windows

"**/node_modules/*/**": true

macOS/Linux

"**/node_modules/**": true,

I recommend using the new default setting on macOS/Linux

"**/node_modules/**/*": true

node-project-1

vscodebot[bot] commented 3 years ago

(Experimental duplicate detection) Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

bpasero commented 3 years ago

Reproduces only when the lock files exist top level, otherwise we refresh the explorer because the lock files appear.

bpasero commented 3 years ago

I first have to understand why we decided to not update the glob for macOS/Linux, there was a reason...

isidorn commented 3 years ago

@bpasero are the file events simply missing? Can I help from the Explorer side?

bpasero commented 3 years ago

@isidorn well for some historic reason we decided to exclude file events on node_modules folder but only on macOS and Linux, so when you do an npm install the folder does not become visible in some cases. I think the explorer is not doing anything wrong, it is just our default watcher exclude glob pattern being quite restrictive.

bpasero commented 3 years ago

Here are some findings how the exclude setting is used:

Windows We use a C# based file watcher that reports every file change unfiltered. We do the filtering using our (!) glob library in the renderer.

macOS/Linux: folder workspace We use chokidar and pass the excludes directly to the library as ignored patterns. This bypasses our glob library and relies on anymatch: https://github.com/paulmillr/chokidar#path-filtering

macOS/Linux: multi-root workspace We use nsfw watcher library. Similar to Windows, that library reports every change unfiltered. We do filtering in our glob library.

Remote Server We use chokidar but do not pass the excludes to the library. We do filtering in our glob library. @aeschli I think you added this, please be aware that we may be changing the default watcherExclude setting slightly to report changes on the node_modules folder (not all, just the top level folders).

isidorn commented 3 years ago

Why don't we also exclude file events on node_modules on windows? Because the file watching works best on that platform?

I think the ideal experience is that when an user does an npm install that the node_modules folder becomes visible, but the file changes inside of it are being ignored...

bpasero commented 3 years ago

@isidorn we do filter on Windows, the change in https://github.com/microsoft/vscode/issues/23954 was specifically to keep the filtering as it was but allow for changes to the node_modules folder itself to not be filtered. Basically what we want is:

The last point would help in the case where a user does yarn add xy and has the node modules folder expanded to see changes coming in. But I would say that is a nice to have, more importantly we need to make sure the node modules folder itself appears when it does.

bpasero commented 3 years ago

Verification (macOS, or Linux):

isidorn commented 3 years ago

Works fine. Even folders inside node_modules appear -> verified