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 should return path not undefined #46915

Closed
evenstensberg opened this issue Mar 2, 2023 · 7 comments
Closed

fs:mkdir should return path not undefined #46915

evenstensberg opened this issue Mar 2, 2023 · 7 comments
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.

Comments

@evenstensberg
Copy link

What is the problem this feature will solve?

When switching from mkdirp we should allow fs.mkdir return the path, not undefined.

What is the feature you are proposing to solve the problem?

Make fs.mkdir return the path

What alternatives have you considered?

N/A

@evenstensberg evenstensberg added the feature request Issues that request new features to be added to Node.js. label Mar 2, 2023
@evenstensberg
Copy link
Author

I'd like to fix this if you can point me to the correct code

@jakecastelli
Copy link
Member

Hi @evenstensberg do you have a code snippet to demonstrate that it is returning undefined? cheers

@jakecastelli
Copy link
Member

jakecastelli commented Mar 3, 2023

I can see from the doc, it explicitly mentioned that

path Present only if a directory is created with recursive set to true.

I think there might be a reason for this behaviour, I'd suggest you wait a little bit until we can hear from the node/fs team or other maintainers.

reference: lib/fs, c++ binding

@meixg
Copy link
Member

meixg commented Mar 3, 2023

Related to #43015

@meixg
Copy link
Member

meixg commented Mar 3, 2023

Seems that the path is added to solve a problem with recursive: true: #31530.
And since only one folder will be made without recursive: true, returning path does not provide more information.

I do agree we can return path to make it more consistent though.

@VoltrexKeyva VoltrexKeyva added the fs Issues and PRs related to the fs subsystem / file system. label Mar 4, 2023
@bnoordhuis
Copy link
Member

No one has explicitly said it so far but the ask is to change the callback in fs.mkdir(loc, (err, path) => { /* ... */}) so that path is always a string, never undefined?

That's a backwards incompatible change from the documented behavior (i.e., likely to break existing code) and we don't usually make those unless the benefits clearly outweigh the drawbacks.

Being a little more compatible with a third-party module isn't a compelling enough reason.

@bnoordhuis
Copy link
Member

Since no one chimed in after my last comment I'm going to close this but let me know if there is reason to reopen.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

No branches or pull requests

5 participants