Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

No way to unwatchFile a specific callback #3660

Closed
stechz opened this issue Jul 7, 2012 · 5 comments
Closed

No way to unwatchFile a specific callback #3660

stechz opened this issue Jul 7, 2012 · 5 comments

Comments

@stechz
Copy link

stechz commented Jul 7, 2012

When calling unwatchFile, it seems that all callbacks will no longer be fired. This is bad because:

  1. there are legitimate reasons to remove a specific callback without removing all callbacks
  2. modules that use watchFile may have their files unwatched without any knowledge of it

I'm not sure how to address this without breaking this API. I'm not sure what 3 - Stable precisely means, but is it possible to add an optional extra parameter for a callback?
@stechz
Copy link
Author

stechz commented Jul 7, 2012

Oh, and I'd be happy to write a patch!

bnoordhuis added a commit to bnoordhuis/node that referenced this issue Jul 8, 2012
@bnoordhuis
fs: make unwatchFile() remove a specific listener
d8401ab 
Before this commit, `fs.unwatchFile(path)` removed *all* listeners for `path`.

The function is overloaded now: `fs.unwatchFile(path)` still removes all
listeners, but `fs.unwatchFile(path, cb)` lets you remove a specific listener.

Fixes nodejs#3660.
@bnoordhuis
Copy link
Member

When calling unwatchFile, it seems that all callbacks will no longer be fired.

Fair point. Can someone review bnoordhuis/node@v0.8...issue3660 ?

It's against v0.8 because I consider it a bug fix. If someone strongly objects and has the arguments to back it up, I don't mind landing it in master.

@bnoordhuis bnoordhuis reopened this Jul 8, 2012
@isaacs
Copy link

isaacs commented Jul 9, 2012

@bnoordhuis I agree, this is a bug. Also, it's marked unstable, and is marked for future deprecation, and this is not a breaking change. LGTM.

@bnoordhuis
Copy link
Member

Thanks. Landed in 5b5362a.

@stechz
Copy link
Author

stechz commented Jul 9, 2012

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@isaacs @bnoordhuis @stechz