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

Recursive mkdir to allow path.sep in a path #44

Closed
wants to merge 3 commits into from

Conversation

justinormont
Copy link

From #43:

Prefix which includes a path.sep fails.

Repro

This fails as the path will include a path.sep:

const uuid = 'xyz';
const prefixForRun = path.join('runs', `run_${uuid}`);
const outputFolder = temporaryDirectory({prefix: prefixForRun});

This call results in a path of:
'/private/var/folders/6t/frtzt7q96fv9xjqp3knl3vqh0000gn/T/runs/run_xyz'

The mkdir for run_xyz fails since the parent, /private/var/folders/6t/frtzt7q96fv9xjqp3knl3vqh0000gn/T/runs/, does not exist.

Error

Error: ENOENT: no such file or directory, mkdir '/private/var/folders/6t/frtzt7q96fv9xjqp3knl3vqh0000gn/T/runs/run_xyz'
    at Object.mkdirSync (node:fs:1324:3)

Solution

In mkdir, add the {recursive: true} option.

Discussion

Incidentally, fixing this gives an indirect method of #35, as my intention was similar. To organize my temp files/folders for later cleanup, I was adding a parent folder to my requested temp folder.

Closes #43

@justinormont
Copy link
Author

@sindresorhus: Can you take a look at my short PR?


t.true(temporaryDirectory().includes(tempDir));
t.true(path.basename(temporaryDirectory({prefix})).startsWith(prefix.split(path.sep)[1]));
t.true(path.dirname(temporaryDirectory({prefix})).split(path.sep).pop() === prefix.split(path.sep)[0]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to put this into a separate test.


t.true(temporaryDirectory().includes(tempDir));
t.true(path.basename(temporaryDirectory({prefix})).startsWith(prefix.split(path.sep)[1]));
t.true(path.dirname(temporaryDirectory({prefix})).split(path.sep).pop() === prefix.split(path.sep)[0]);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use t.is

@sindresorhus
Copy link
Owner

I don't think this is a good idea. What if you want just a parent directory and not a name prefix. That becomes awkward. I think it would be better to introduce a new option that worked for both file and directory called parentDirectory.

@sindresorhus
Copy link
Owner

Closing for lack of activity.

#46

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.

Prefix which includes a path.sep fails
2 participants