Node: feature-request fs.fileWrite with option.mkdirp

[kaizhu256]

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;
        }
    };

[devsnek]

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.

[kaizhu256]

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.

[devsnek]

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

[kaizhu256]

yes, common-case is far fewer directories than files

[benjamingr]

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?

[kaizhu256]

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
        );

[rickyes]

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

[ConorDavenport]

I'd like to give this a shot

[bcoe]

@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.

[kaizhu256]

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

1. have no performance-impact on fs.writeFile in common-case where mkdirp is not required
2. 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

[kaizhu256]

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);
     });

[bcoe]

@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).