-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Use the Node.js 10.12 recursive
options when available
#5
Conversation
Tests are failing. |
They failing on my ubuntu with no modification as well.
I see:
|
You're right. Hmm, weird. Not sure why master branch is suddenly failing. Something must have changed in recent Node.js patch versions... Any ideas? |
recursive
options when available
I have no idea. I do not understand what are you checking with this tests. Is it mode of a root dir? But what for, |
Fixed tests, as I understood a problem was with root mode which should be passed directly, because it is not |
Your change makes the test fail on Node.js 8. Something changed in Node.js 10. I'm just not sure exactly what. |
The tests are fixed in master branch. Can you fix the merge conflict? |
Done. |
@@ -31,7 +31,10 @@ module.exports = (input, opts) => Promise.resolve().then(() => { | |||
const stat = pify(opts.fs.stat); | |||
|
|||
const make = pth => { | |||
return mkdir(pth, opts.mode) | |||
return mkdir(pth, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a problem because someone could set opts.fs containing an alternate implementation mkdir.
I think you can only use mkdir(pth, options_object) only when node.js is at least 10.12 and opts.fs.mkdir === fs.mkdir. In all other cases the legacy mkdir(pth, opts.mode) method needs to be used.
The same comment applies to the sync
method, it needs to check the version and verify opts.fs.mkdirSync === fs.mkdirSync before using the options object.
In Node v10.12 a new flag
recursive
was introduced to fs.mkdir.Looks like
c++
implementation 38% faster thenjs
.Would be great to have ability to benefit from it using
make-dir
onnode > v10.12
and avoid additional checks and feature detecting for lover node versions :).