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

Using graceful-ncp instead of ncp #95

Closed
wants to merge 1 commit into from

Conversation

adam-lynch
Copy link

This helps with EMFILE errors. See adam-lynch/graceful-ncp.

This is related to nwutils/nw-builder#123

@adam-lynch
Copy link
Author

ping

@jprichardson
Copy link
Owner

I'm in favor of using graceful-fs... but I don't see the need for a new module to merge the two. Why not submit a PR to ncp to optionally use graceful-fs much like this module does?

@adam-lynch
Copy link
Author

The maintainer is serious about "not taking any dependencies into this module".

One place where he has said it: AvianFlu/ncp#16.

@jprichardson
Copy link
Owner

Yes, I saw that. It's not necessary to take it as a dependency. See this module as an example. It's just a matter of:

try {
  fs = require("graceful-fs")
} catch (er) {
  fs = require("fs")
}

No dependency required.

@adam-lynch
Copy link
Author

Sure, I'll try. I assumed it wouldn't be accepted.

@adam-lynch
Copy link
Author

Forgot to post the PR here; AvianFlu/ncp#70

@traviswimer
Copy link

Another possibility that might work better if the dependency is unacceptable is to just allow fs to be passed as a parameter. In other words something like this maybe:

var gfs = require("graceful-fs");
var fs = require("fs-extra")( gfs );

I didn't actually look at the code to know if this specifically is reasonable, but it would be nice to have the option of the library of you choice through some process.

@jprichardson
Copy link
Owner

@traviswimer graceful-fs is already a part of fs-extra and has been for awhile, see: https://github.com/jprichardson/node-fs-extra/blob/master/lib/index.js#L8 The concern is that ncp, the copy module that fs-extra uses does not use graceful-fs and won't accept any dependency modules.

Either way, I think I'm going to fork ncp into fs-extra as the ncp guys have been pretty silent lately.

@jprichardson
Copy link
Owner

@jprichardson
Copy link
Owner

Includes graceful-fs now as well.

@adam-lynch
Copy link
Author

Could you clarify what impact this has? What do you mean by "io.js only"? Is this a backwards-incompatibile change? What I mean is will I have to switch to io.js everywhere my code is ran for this to work? I haven't read into io.js enough yet probably.

@jprichardson
Copy link
Owner

@adam-lynch 0.15.0 includes ncp, uses graceful-fs, patches ncp for support for io.js. You won't have to modify any of your code as it works on both node.js and io.js.

@adam-lynch
Copy link
Author

Thanks

On Wed, 21 Jan 2015 22:52 JP Richardson [email protected] wrote:

@adam-lynch https://github.com/adam-lynch 0.15.0 includes ncp, uses
graceful-fs, patches ncp for support for io.js. You won't have to modify
any of your code as it works on both node.js and io.js.


Reply to this email directly or view it on GitHub
#95 (comment)
.

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.

3 participants