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

Add support for fs.watch(). #300

Closed
wants to merge 1 commit into from
Closed

Conversation

dpryden
Copy link

@dpryden dpryden commented Jun 22, 2020

This support currently only works for Node v10 and newer, since the
fs_event_wrap native binding is lazy-loaded and thus can be pre-patched
before first use.

To make this robust enough to actually use, it may be necessary to also
add in monkeypatching of the fs.watch endpoint.

The tests are incomplete; mainly just enough to demonstrate that this
works as a proof of concept.

This support currently only works for Node v10 and newer, since the
fs_event_wrap native binding is lazy-loaded and thus can be pre-patched
before first use.

To make this robust enough to actually use, it may be necessary to also
add in monkeypatching of the fs.watch endpoint.

The tests are incomplete; mainly just enough to demonstrate that this
works as a proof of concept.
@tschaub
Copy link
Owner

tschaub commented Aug 20, 2020

Thanks for the work on this, @dpryden. I didn't intend to close this pull request – forgot that that happens automatically when changing default branches. We can still use these commits if someone else wants to take up this work. Or feel free to open a new pull request (you should be able to git pull --rebase upstream main && git push --force origin fs-watch depending on how you have named your remotes).

@dpryden
Copy link
Author

dpryden commented Aug 21, 2020

OK, I rebased (and fixed a minor merge conflict) and created #308 as a replacement. Thanks for reminding me about this PR @tschaub !

I no longer have a pressing need for this change but I'm happy to help move it forward. However, realistically I think it might be easiest to wait until mock-fs is willing to drop support for old versions of Node... and I have no idea when that might happen, or if it's worth investing more effort in this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants