feature-request fs.fileWrite with option.mkdirp

Is your feature request related to a problem? Please describe.

i am author of npm-package istanbul-lite.
when creating coverage-reports and artifacts,
its common to lazily write data to file-directories that have yet to be created.

Describe the solution you'd like

add extra boolean <options>.mkdirp to functions fs.writeFile and fs.writeFileSync,
that will lazily create missing file-directories when writing to file.
the common-use-case are:

lazily generate directories when writing coverage-report of instrumented files
lazily generate directories when writing artifacts in ci and testing
lazily generate directories when scaffolding new web-projects

Describe alternatives you've considered

i currently use this helper-function in all my projects,
to lazily generate directories when writing artifacts and coverage-reports.

    function fsWriteFileWithMkdirpSync (file, data) {
    /*
     * this function will sync write <data> to <file> with "mkdir -p"
     */
        let fs;
        // do nothing if module does not exist
        try {
            fs = require("fs");
        } catch (ignore) {
            return;
        }
        // try to write file
        try {
            fs.writeFileSync(file, data);
            return true;
        } catch (ignore) {
            // mkdir -p
            fs.mkdirSync(require("path").dirname(file), {
                recursive: true
            });
            // rewrite file
            fs.writeFileSync(file, data);
            return true;
        }
    };

Given that you can just do this (recursive mkdir doesn't throw if the directory already exists):

fs.mkdirSync(path.dirname(file), { recursive: true });
fs.writeFileSync(file, data);

I don't personally think adding the option is needed.

calling fs.mkdirSync(path.dirname(file), { recursive: true }); is expensive,
when generating 100+ artifacts/coverage-reports/scaffolding.

its more efficient to first attempt a normal fs.writeFile in try-catch, and lazily mkdir in the uncommon-case that directory doesn't exist.

I assume you don't have 100+ output directories though?

yes, common-case is far fewer directories than files

This sounds pretty useful and I am not opposed in principle. I tried looking for it elsewhere but most libraries I saw just mkdirSync'd and writeFileSync'd. Can you maybe share some examples from libraries in the ecosystem that do this?

yea, i'm only aware of myself following practice of lazy-mkdirp-after-failed-write.

it seems official istanbuljs will eager mkdirSync before writing to file (https://github.com/istanbuljs/istanbuljs/blob/65b1331b52ca92f2aa3e7c41218698a57cba6a42/packages/istanbul-lib-report/lib/file-writer.js#L184)

but here are code-snippets from my own public-projects that will lazy-mkdirp-after-failed-write:

create html-coverage-report in arbitrary sub-directory dictated by instrumented-file's pathname

// https://github.com/kaizhu256/node-istanbul-lite/blob/63044848968528d677591e352d063eae8c76ada1/lib.istanbul.js#L11554
    writeFile: function (file, onError) {
        local.coverageReportHtml += local.writerData + "\n\n";
        if (!local.isBrowser && local.writerFile) {
            local.fsWriteFileWithMkdirpSync(local.writerFile, local.writerData);
        }
        local.writerData = "";
        local.writerFile = file;
        onError(local.writer);
    }

create test-report artifact

// https://github.com/kaizhu256/node-utility2/blob/0c44e424ce0bd20d7fc03e4711eeaaa888cbb166/lib.utility2.js#L6676
    // create test-report.html
    local.fsWriteFileWithMkdirpSync(
        "tmp/build/test-report.html",
        local.testReportMerge(testReport)
    );

test webserver by running it on local-port, download its assets, and save them as artifacts:

// https://github.com/kaizhu256/node-utility2/blob/0c44e424ce0bd20d7fc03e4711eeaaa888cbb166/lib.utility2.js#L3212
    local.onParallelList({
        list: [
            {
                file: "/LICENSE",
                url: "/LICENSE"
            }, {
                file: "/assets." + local.env.npm_package_nameLib + ".css",
                url: "/assets." + local.env.npm_package_nameLib + ".css"
            }, {
                file: "/assets." + local.env.npm_package_nameLib + ".html",
                url: "/index.html"
            }, {
                file: "/assets." + local.env.npm_package_nameLib + ".js",
                url: "/assets." + local.env.npm_package_nameLib + ".js"
...
        xhr = await local.httpFetch(local.serverLocalHost + opt2.url, {
            responseType: "raw"
        });
...
        local.fsWriteFileWithMkdirpSync(
            "tmp/build/app" + opt2.file,
            xhr.data
        );

Perhaps a function-type parameter can be provided, which is executed when the file opening fails.

I'd like to give this a shot

@kaizhu256

@kaizhu256 I'm on the fence, writing:

fs.mkdirSync(path.dirname(file), { recursive: true });
fs.writeFileSync(file, data);

Isn't too much of a hassle. However, I do appreciate us eliminating toil where possible.

Can you find an example of any other platforms that have this functionality, sometimes if I can't make my mind up about a feature, I find it useful to look at the standard libraries for a few other languages.

@bcoe

@bcoe, the toil is actually greater, because as end-user, i want:

have no performance-impact on fs.writeFile in common-case where mkdirp is not required
silently ignore ENOENT error

// 1. have no performance-impact on fs.writeFile in common-case where mkdirp is not required
// by first attempting to fs.writeFile[Sync] with no mkdir
try {
    fs.writeFileSync(file, data);
} catch (errCaught) {
    // 2. silently ignore `ENOENT` error
    // and lazy-mkdirp after common-case fails
    // (where performance is usually not critical)
    if (errCaught?.code === 'ENOENT') {
        fs.mkdirSync(path.dirname(file), { recursive: true });
        fs.writeFileSync(file, data);
        return;
    }
    throw errCaught;
}

unfortunately, i'm not aware of file-write-with-lazy-mkdirp in other platforms. i do use this feature however, in my npm-packages istanbul-lite (create coverage-artifacts) and utility2 (create test-reports and build-artifacts):

https://github.com/kaizhu256/node-istanbul-lite/blob/2020.11.12/lib.istanbul.js#L408
https://github.com/kaizhu256/node-utility2/blob/2020.11.12/lib.utility2.js#L4054

the closest thing i'm aware of is posix "mkdir -p" (which ignores ENOENT):

#!/bin/sh
mkdir -p /tmp/aa/bb/ && printf 'hello world!\n' > /tmp/aa/bb/foo.txt

here's possible real-world application to npm-package istanbul-reports
removing external-dependency make-dir, and replacing it with built-in fs.writeFileSync and fs.openWithMkdirpSync:

# remove-dependency-make-dir.diff

diff --git a/monorepo-merge-reports.js b/monorepo-merge-reports.js
index 74a5497..f22e13a 100644
--- a/monorepo-merge-reports.js
+++ b/monorepo-merge-reports.js
@@ -5,12 +5,11 @@ const path = require('path');
 const { spawnSync } = require('child_process');

 const rimraf = require('rimraf');
-const makeDir = require('make-dir');
 const glob = require('glob');

 process.chdir(__dirname);
 rimraf.sync('.nyc_output');
-makeDir.sync('.nyc_output');
+require('fs').mkdirSync('.nyc_output');
 // Merge coverage data from each package so we can generate a complete report
 glob.sync('packages/*/.nyc_output').forEach(nycOutput => {
     const cwd = path.dirname(nycOutput);
diff --git a/packages/istanbul-lib-report/lib/file-writer.js b/packages/istanbul-lib-report/lib/file-writer.js
index de1154b..c071173 100644
--- a/packages/istanbul-lib-report/lib/file-writer.js
+++ b/packages/istanbul-lib-report/lib/file-writer.js
@@ -5,7 +5,6 @@
  */
 const path = require('path');
 const fs = require('fs');
-const mkdirp = require('make-dir');
 const supportsColor = require('supports-color');

 /**
@@ -157,14 +156,13 @@ class FileWriter {
             throw new Error(`Cannot write to absolute path: ${dest}`);
         }
         dest = path.resolve(this.baseDir, dest);
-        mkdirp.sync(path.dirname(dest));
         let contents;
         if (header) {
             contents = header + fs.readFileSync(source, 'utf8');
         } else {
             contents = fs.readFileSync(source);
         }
-        fs.writeFileSync(dest, contents);
+        fs.writeFileSync(dest, contents, { mkdirp: true });
     }

     /**
@@ -181,8 +179,7 @@ class FileWriter {
             throw new Error(`Cannot write to absolute path: ${file}`);
         }
         file = path.resolve(this.baseDir, file);
-        mkdirp.sync(path.dirname(file));
-        return new FileContentWriter(fs.openSync(file, 'w'));
+        return new FileContentWriter(fs.openWithMkdirpSync(file, 'w'));
     }
 }

diff --git a/packages/istanbul-lib-report/package.json b/packages/istanbul-lib-report/package.json
index 91cb16e..3cd8783 100644
--- a/packages/istanbul-lib-report/package.json
+++ b/packages/istanbul-lib-report/package.json
@@ -13,7 +13,6 @@
   },
   "dependencies": {
     "istanbul-lib-coverage": "^3.0.0",
-    "make-dir": "^3.0.0",
     "supports-color": "^7.1.0"
   },
   "devDependencies": {
diff --git a/packages/istanbul-lib-report/test/file-writer.test.js b/packages/istanbul-lib-report/test/file-writer.test.js
index 707785b..843e3a8 100644
--- a/packages/istanbul-lib-report/test/file-writer.test.js
+++ b/packages/istanbul-lib-report/test/file-writer.test.js
@@ -4,7 +4,6 @@
 const fs = require('fs');
 const path = require('path');
 const assert = require('chai').assert;
-const mkdirp = require('make-dir');
 const rimraf = require('rimraf');
 const FileWriter = require('../lib/file-writer');

@@ -14,7 +13,6 @@ describe('file-writer', () => {
     let writer;

     beforeEach(() => {
-        mkdirp.sync(dataDir);
         writer = new FileWriter(dataDir);
     });

@iansu

@iansu and @joesepi have been working on a document outlining some of the future improvements to fs operations we'd like to make, as part of the @nodejs/tooling group (perhaps they could chime in too).

fs: add option `mkdirp` to fs.writeFile[Sync], fs.appendFile[Sync]

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

Navigation Menu

Repository navigation

feature-request fs.fileWrite with option.mkdirp #33559

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

feature-request fs.fileWrite with option.mkdirp #33559

Description

Activity

devsnek commented on May 25, 2020

kaizhu256 commented on May 25, 2020

devsnek commented on May 25, 2020

kaizhu256 commented on May 25, 2020

benjamingr commented on May 25, 2020

kaizhu256 commented on May 26, 2020

rickyes commented on May 26, 2020

DavenportEmma commented on May 26, 2020

bcoe commented on Nov 16, 2020

kaizhu256 commented on Nov 16, 2020

kaizhu256 commented on Nov 16, 2020

bcoe commented on Nov 17, 2020

github-actions commented on Mar 18, 2022

github-actions commented on Apr 18, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions