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

fs.mkdir is inconsistent in the number of arguments passed to the callback , when completed. #43015

Closed
ryan65 opened this issue May 9, 2022 · 3 comments · Fixed by #43016
Closed
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@ryan65
Copy link

ryan65 commented May 9, 2022

Version

16.14

Platform

Window/Linux

Subsystem

No response

What steps will reproduce the bug?

//call mkdir with to create a non existing directory and then when complete call it again with the same directory.
fs.mkdir('c:\\temp\\testdirnonexist',{ recursive: true }, function(){
       console.log('first time', arguments.length); //result 2
       fs.mkdir('c:\\temp\\testdirnonexist',{ recursive: true }, function(){
          console.log('second time', arguments.length); //result 1
       });
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Always return the same number of arguments.
This is important for example when you pass an async waterfall callback to mkdir.

What do you see instead?

if the directory exists only 1 arguments will be returned (error argument which is null)
if directory is created then it returns null, and the path.

Additional information

No response

daeyeon added a commit to daeyeon/node that referenced this issue May 9, 2022
fixes: nodejs#43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
@daeyeon
Copy link
Member

daeyeon commented May 9, 2022

https://github.com/nodejs/node/blob/master/doc/api/fs.md#fsmkdirpath-options-callback
According to the document, it seems a correct behavior but there is a missing description for the parameter. I will create a PR to improve it.

The callback is given a possible exception and, if recursive is true, the first directory path created, (err[, path]). path can still be undefined when recursive is true, if no directory was created.

@daeyeon
Copy link
Member

daeyeon commented May 9, 2022

Please let me know if the doc fix isn't enough to resolve your issue.

@VoltrexKeyva VoltrexKeyva added the fs Issues and PRs related to the fs subsystem / file system. label May 9, 2022
nodejs-github-bot pushed a commit that referenced this issue May 22, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
@ryan65
Copy link
Author

ryan65 commented May 22, 2022

Hi
Thanks Daeyeon
I think the documentation now will be clearer, although I personally am still not sure what the most intuitive behavior should
be, regarding the returned result, since currently its inconsistent. but the documentation at least clarifies this.

bengl pushed a commit that referenced this issue May 30, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
juanarbol pushed a commit that referenced this issue May 31, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
danielleadams pushed a commit that referenced this issue Jun 27, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
targos pushed a commit that referenced this issue Jul 12, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
targos pushed a commit that referenced this issue Jul 31, 2022
fixes: #43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43016
Fixes: #43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
fixes: nodejs/node#43015

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: nodejs/node#43016
Fixes: nodejs/node#43015
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants