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

Non recursive watch requests are not refcounted #151916

Closed
taronlee opened this issue Jun 13, 2022 · 10 comments
Closed

Non recursive watch requests are not refcounted #151916

taronlee opened this issue Jun 13, 2022 · 10 comments
Assignees
Labels
file-watcher File watcher
Milestone

Comments

@taronlee
Copy link

taronlee commented Jun 13, 2022

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

  • VS Code Version: Version: 1.68.0 (user setup)
    Commit: 4af164e
    Date: 2022-06-08T11:44:16.822Z
    Electron: 17.4.7
    Chromium: 98.0.4758.141
    Node.js: 16.13.0
    V8: 9.8.177.13-electron.0

  • OS Version: Windows_NT x64 10.0.17134

Steps to Reproduce:
We have .gbs.conf file and watch this file using FileSystemWatcher.

this.fsProvider = workspace.createFileSystemWatcher(
      new RelativePattern(homedir(), '.gbs.conf')
);

this.disposable.push(
  this.fsProvider.onDidChange(async e => {
    console.log(`onDidChange event`);        
  })
);

when I set .gbs.conf as pattern parameter, onDidChange event fires twice each time.

@vscodenpa
Copy link

Thanks for creating this issue! It looks like you may be using an old version of VS Code, the latest stable release is 1.68.0. Please try upgrading to the latest version and checking whether this issue remains.

Happy Coding!

@bpasero
Copy link
Member

bpasero commented Jun 13, 2022

File events are not guaranteed to be single, so it is totally expected that you get 2 or even more events. For example, file writes via node.js are often a truncate first and then a write, which can emit 2 events.

How do you change the file? Is this native windows, or WSL?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jun 13, 2022
@taronlee
Copy link
Author

How do you change the file? Is this native windows, or WSL?
=> WSL, I am using remote-ssh. (windows -> build server (ubuntu))

in our project, we use FileSystemWatcher in many case.
but only in this case ( file name starts with .), emit 2 events each time.

@bpasero
Copy link
Member

bpasero commented Jun 16, 2022

Do you see the same issue when using Windows file system without WSL?

@taronlee
Copy link
Author

Hello,
I tested it on Windows without WSL, but still emit 2 events each time.

onDidChange event
onDidChange event

@taronlee
Copy link
Author

when I set **.gbs.conf as pattern parameter, onDidChange event fires just one each time.

But I'm worried that it doesn't exactly match the file name.

I think setting .gbs.conf as pattern parameter should work just fine too.

@bpasero bpasero added this to the June 2022 milestone Jun 23, 2022
@bpasero bpasero added the file-watcher File watcher label Jun 23, 2022
@bpasero
Copy link
Member

bpasero commented Jun 23, 2022

And the 2 events are reported when you use VSCode to make a change to the file?

@taronlee
Copy link
Author

yes, I made a change to the file using VSCode.
It doesn't matter where I make a change.

@bpasero bpasero removed the info-needed Issue requires more information from poster label Jun 23, 2022
@bpasero
Copy link
Member

bpasero commented Jun 23, 2022

I tested this with bare node.js and get the same result, so this is standard node.js behaviour:

const fs = require("fs");

fs.watch("C:\\Users\\bpasero\\Desktop\\empty\\new.js", console.log);

fs.writeFileSync("C:\\Users\\bpasero\\Desktop\\empty\\new.js", 'Hello World');

image

When installing a recursive file watcher (via complex glob pattern, e.g. including **), we install a different file watcher that is probably not impacted: https://github.com/parcel-bundler/watcher

Unfortunately that watcher does not support flat (non-recursive) file watching, otherwise we would never use node.js watcher. I had asked for this in parcel-bundler/watcher#92

Closing as designed / upstream, sorry.

@bpasero bpasero closed this as completed Jun 23, 2022
@bpasero bpasero added upstream Issue identified as 'upstream' component related (exists outside of VS Code) nodejs NodeJS support issues labels Jun 23, 2022
@bpasero bpasero reopened this Jun 23, 2022
@bpasero bpasero changed the title FileSystemWacther event fires twice each time. Non recursive watch requests are not refcounted Jun 23, 2022
@bpasero bpasero removed upstream Issue identified as 'upstream' component related (exists outside of VS Code) nodejs NodeJS support issues labels Jun 23, 2022
@bpasero
Copy link
Member

bpasero commented Jun 23, 2022

Nevermind, while it is true that node.js emits 2 events out of the box, we actually have logic to deduplicate events in a time frame of 50ms.

The issue here is different:

  • the extension installs a watcher using the parent folder and the file name as pattern
  • when you open the file in vscode, we also install a watcher to react to changes in the editor
  • as such, any change you do ends up with 2 events

Let's move into #153009 as a more general issue. For now there is no good solution other than deduplicating on your end, sorry.

@bpasero bpasero closed this as completed Jun 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Aug 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
file-watcher File watcher
Projects
None yet
Development

No branches or pull requests

3 participants