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

fsPromises.mkdir() returns an incomplete path when the recursive option is passed #40829

Open
bnb opened this issue Nov 16, 2021 · 7 comments
Open
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@bnb
Copy link
Contributor

bnb commented Nov 16, 2021

Version

17.0.0

Platform

Linux failsafe 5.10.16.3-microsoft-standard-WSL2 #1 SMP Fri Apr 2 22:23:49 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

file system

What steps will reproduce the bug?

const { mkdir } = require('fs/promises');
const { resolve } = require('path');

async function makeDirectory () {
  const path = resolve('./test/project/lol/hi')
  const dirCreation = await mkdir(path, { recursive: true });
  
  return dirCreation;
}

console.log(makeDirectory());

will log/return /home/bnb/GitHub/tmp/promisesmkdir/test rather than /home/bnb/GitHub/tmp/promisesmkdir/test/project/lol/hi, despite actually creating the latter.

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

100% of the time as far as I can tell.

What is the expected behavior?

Output the full path.

What do you see instead?

A partial path, seemingly just the first directory that was created rather than every directory that was created.

Additional information

<3

@bnb bnb changed the title fsPromises.mkdir() returns all but the last fsPromises.mkdir() returns an incomplete path when the recursive option is passed Nov 16, 2021
@targos
Copy link
Member

targos commented Nov 16, 2021

It behaves as documented. Is there a problem with this behavior?

@bnb
Copy link
Contributor Author

bnb commented Nov 16, 2021

It does behave as documented. That behavior is also not particularly helpful. For example, if I want to create a directory recursively, await that happening, and then act on that created directory I need to store the directory I created somehow (in a variable or some other way) and then re-use that when I get this partial path back as confirmation.

There's a few options that would be better than the current API:

  • returning the full path
  • returning an object that has the full path, an indicator of whether or not the path was created, and any other useful information
  • returning a boolean rather than undefined OR a partial string

@bnb
Copy link
Contributor Author

bnb commented Nov 16, 2021

Also: I misread the docs (I missed the word first) so I suppose this is a feature request though I'd honestly consider this a bug since it's not materially useful without more work than should be necessary.

@Mesteery Mesteery added the fs Issues and PRs related to the fs subsystem / file system. label Nov 16, 2021
@bcoe
Copy link
Contributor

bcoe commented Nov 19, 2021

@bnb @targos, I believe I originally had this implemented as described (always returning the deepest path), and the complaint was that there was no way to tell which directory had actually been created.

I believe the current behavior is to return the first directory created? so you can work backwards and infer exactly which directories didn't exist.

@bnb
Copy link
Contributor Author

bnb commented Nov 19, 2021

Yep, that's the current behavior. I can understand that reasoning but thinking about how I'd actually use this (await the result of mkdir and then use what it returns to write a file to, for example), I'd have to do significantly less logic myself if it provided the full path. I've not found a case where I really care which part of the path was created, I just want to consume the result.

Perhaps an option would be to change what it returns to an object that includes this information?

@bcoe
Copy link
Contributor

bcoe commented Nov 19, 2021

@bnb could you do this:

const { mkdir } = require('fs/promises');
const { resolve } = require('path');

async function makeDirectory () {
  const path = resolve('./test/project/lol/hi')
  const dirCreation = await mkdir(path, { recursive: true });
  return path;
}

console.log(await makeDirectory());

And if an exception occurs you know something went wrong creating the path.

It would be a breaking change of a stable feature to change the return, and I'm pretty sure folks are relying on the current behavior. An option would be to add an option flag to return an alternate value, I'd just want to make sure that it's worth it and there's not a workaround people can instead use.

@bnb
Copy link
Contributor Author

bnb commented Nov 29, 2021

An option would be to add an option flag to return an alternate value, I'd just want to make sure that it's worth it and there's not a workaround people can instead use.

I mean that works, but imo isn't something we should really be forcing end-users to have to work around. I'm also not super keen on changing what's returned based on an option, so I'm not sure if there's a path forward here outside of me requesting a semver major breaking change to make it return an Object with the current value in addition to some others or the full path as a string (the latter, I assume, would be less obviously breaking and therefore more painful).

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

No branches or pull requests

4 participants