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.

Sign up for GitHub

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

Jump to bottom

fs: add .ref() and .unref() methods to the StatWatcher and FSWatcher #33134

Closed
wants to merge 3 commits into from
Closed

fs: add .ref() and .unref() methods to the StatWatcher and FSWatcher #33134

wants to merge 3 commits into from

Conversation

rickyes
Copy link
Contributor

@rickyes rickyes commented Apr 28, 2020

add .ref() and .unref() methods to the StatWatcher and FSWatcher classes.

Fixes: #33096

/cc @addaleax

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. labels Apr 28, 2020
@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 28, 2020
addaleax
addaleax reviewed Apr 28, 2020
View reviewed changes
doc/api/fs.md Show resolved Hide resolved
doc/api/fs.md
event loop to remain active. If there is no other activity keeping the
event loop running, the process may exit before the `FSWatcher` object's
callback is invoked. Calling `watcher.unref()` multiple times will have
no effect.
Copy link
Member

Choose a reason for hiding this comment

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

These methods should probably also be documented for the return value of .watchFile().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, there are other methods like .stop(), I'm not sure if it should be documented in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

lib/fs.js Outdated Show resolved Hide resolved
@rickyes
Copy link
Contributor Author

rickyes commented Apr 30, 2020

@addaleax @jasnell I've made some new ones. Please review.

jasnell
jasnell reviewed Apr 30, 2020
View reviewed changes
doc/api/fs.md Show resolved Hide resolved
@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 7, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31223/

addaleax pushed a commit that referenced this pull request May 9, 2020
@rickyes @addaleax
fs: add .ref() and .unref() methods to watcher classes
be7fd2d 
PR-URL: #33134
Fixes: #33096
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax
Copy link
Member

addaleax commented May 9, 2020

Landed in be7fd2d :)

@addaleax addaleax closed this May 9, 2020
@rickyes rickyes deleted the add-unref-ref-in-fswatch branch May 9, 2020 06:00
codebytere pushed a commit that referenced this pull request May 11, 2020
@rickyes @codebytere
fs: add .ref() and .unref() methods to watcher classes
1dcf66c 
PR-URL: #33134
Fixes: #33096
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere added a commit that referenced this pull request May 18, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
2324d42 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) remove obsolete completer variable (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) improve repl autocompletion for require calls (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) replace hard coded core module list with actual list (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370
test:
  * (SEMVER-MINOR) refactor test/parallel/test-bootstrap-modules.js (Ruben Bridgewater) #33282

PR-URL: TODO
@codebytere codebytere mentioned this pull request May 18, 2020
Merged
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
76a52dd 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: TODO
codebytere added a commit that referenced this pull request May 19, 2020
@codebytere
2020-05-19, Version 14.3.0 (Current)
a33e87c 
Notable changes:

async_hooks:
  * (SEMVER-MINOR) move PromiseHook handler to JS (Stephen Belanger) #32891
cli:
  * (SEMVER-MINOR) add `--trace-atomics-wait` flag (Anna Henningsen) #33292
fs:
  * (SEMVER-MINOR) add .ref() and .unref() methods to watcher classes (rickyes) #33134
http:
  * (SEMVER-MINOR) expose http.validate-header-name/value (osher) #33119
repl:
  * (SEMVER-MINOR) deprecate repl._builtinLibs (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) deprecate repl.inputStream and repl.outputStream (Ruben Bridgewater) #33294
  * (SEMVER-MINOR) show reference errors during preview (Ruben Bridgewater) #33282
  * (SEMVER-MINOR) improve repl preview (Ruben Bridgewater) #33282
src:
  * add support for TLA (Gus Caplan) #30370

PR-URL: TODO
This was referenced Jul 25, 2021
Merged
Open
Merged
Merged
Merged
Closed
This was referenced Aug 2, 2021
Closed
Closed
@gary-kim-bot gary-kim-bot mentioned this pull request Aug 2, 2021
Open
1 task
@renovate renovate bot mentioned this pull request Aug 3, 2021
Merged
1 task
This was referenced Aug 11, 2021
Closed
Open
@Dmarch28 Dmarch28 mentioned this pull request Jun 19, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Jun 19, 2022
Open
@137717unity 137717unity mentioned this pull request Jun 19, 2022
Open
@baby636 baby636 mentioned this pull request Jun 19, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Jun 19, 2022
Open
@kissofire kissofire mentioned this pull request Jun 19, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Jun 19, 2022
Open
@ron-a20 ron-a20 mentioned this pull request Jun 19, 2022
Open
@molllyn1 molllyn1 mentioned this pull request Jun 20, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Jan 30, 2023
Open
@wedataintelligence wedataintelligence mentioned this pull request Jan 30, 2023
Open
@snyk-bot snyk-bot mentioned this pull request Jan 30, 2023
Open
@molllyn1 molllyn1 mentioned this pull request Jan 31, 2023
Open
@snyk-bot snyk-bot mentioned this pull request Jan 31, 2023
Open
@baby636 baby636 mentioned this pull request Jan 31, 2023
Open
This was referenced Jan 31, 2023
Open
Open
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
@rickyes @lpinca
fs: add .ref() and .unref() methods to watcher classes
ce9022d 
PR-URL: nodejs/node#33134
Fixes: nodejs/node#33096
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unref FSWatcher
5 participants
@rickyes @nodejs-github-bot @addaleax @MylesBorins @jasnell