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

please tell module authors not to use this in the README #28

Closed
andrewrk opened this issue Jun 1, 2014 · 2 comments
Closed

please tell module authors not to use this in the README #28

andrewrk opened this issue Jun 1, 2014 · 2 comments

Comments

@andrewrk
Copy link

andrewrk commented Jun 1, 2014

This module is awesome. I don't know why it's not just in core node.js all the time.

I often want to require it in my applications. However sometimes I don't want to require it, but I end up requiring it anyway, because 8 module dependencies down the chain, some module has depended on graceful-fs without understanding the consequences.

Can we put a notice in the README that says, "HEY MODULE AUTHORS do not depend on this. This is for applications to depend on only."

For example: EvanOxfeld/node-unzip#54
@andrewrk
Copy link
Author

andrewrk commented Jun 1, 2014

arghh!! it's everywhere! bower/decompress-zip#27

isaacs referenced this issue Jun 2, 2014
@isaacs
Eval is less evil than monkey-patching a global
08471b2 
Closes #27

Due to the way that streams and the various FS methods interact with one
another, NOT monkey-patching doesn't work.

However, since the 'fs' module is effectively a process-level global,
making monkey-patchy changes to it can introduce some really weird behavior.
Even if such behavior is not actively harmful, it can end up being
applied multiple times, and wrapping 'fs.open()' over and over again
is a bit of a bummer when it comes to performance.

Mixing 'eval()' and a private/internal Node API kind of gives me the
heeby jeebies.  If anyone can come up with a less evil way to do this,
I'll be happy to accept a patch.
@isaacs
Copy link
Owner

isaacs commented Jun 2, 2014

Closed by 08471b2

@isaacs isaacs closed this as completed Jun 2, 2014
@isaacs isaacs mentioned this issue Jun 5, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@isaacs @andrewrk