angular/dgeni-packages

No files are written: mkdir@1.0.0 broke mkdir-promise as such writeFile processor broke

dherges opened this issue · 6 comments

dherges commented

mkdirp@1.0.0 changed implementation from callback to promisified api

mkdir-promise has "mkdirp": "*" dependency: https://github.com/ahmadnassri/mkdirp-promise/blob/master/package.json#L37

As such, the writeFile stopped working because the Promise never resolves.

https://github.com/angular/dgeni-packages/blob/master/base/services/writeFile.js#L1

I fixed it locally in a project workspace by overriding the factory with a custom implementation:

import * as mkdirp from 'mkdirp';
import * as fs from 'fs';
import * as path from 'canonical-path';

/**
 * mkdirp module changed implementation from callback to promise in 1.0.0
 *
 * As such, mkdir-promise stopped working.
 *
 * @link https://github.com/isaacs/node-mkdirp/blob/master/CHANGELOG.md
 */
export default function writeFile() {

  return function(file, content) {

    return new Promise((resolve, reject) => {
        mkdirp(path.dirname(file), undefined, (err, made) => {
          if (err === null) {
            resolve(made);
          } else {
            reject(err);
          }
        });
      }).then(() => new Promise((resolve, reject) => {
        fs.writeFile(file, content, function (err) {
          if (err) {
            reject(err);
          }
          resolve();
        });
      }));
  }
}
dherges commented

Thoughts...just thoughs...

  1. Please take this issue to twitter 🤣
  2. Ideally, mkdir-promise should mkdirp instanceof Promise ? mkdir : promisify(mkdirp)... 🤔
  3. It could be worth adding a few promising lines of code for a Promise that's actually a Promise 🐆
petebacondarwin commented

How about we just update dgeni-packages to using mkdirp@1.0 and stop using mkdirp-promise? Since, AFAICT, the new version already returns a promise, right?

dherges commented

Sounds good to me!

petebacondarwin commented

I just got hit by this when updating AIO 🙀
So I better get on and fix it!

  • 👍1
petebacondarwin commented

Thanks for raising this @dherges - it must have been quite a pain to debug what was going on!

  • 👍2
dherges commented

you're welcome!

I had this thought to modify the dgeni code to work with different versions of the dependencies, and that would have required several "if/else" checks...

Just updating the dependencies (mkdirp >= 1.0.0) sounds very good to me!