graceful-fs included by grunt-assemble v0.6.3 breaks system fs.readdir globally

[iosart]

Took me a while to get to the bottom of it, but if you try to run: fs.readdir('./', {withFileTypes: true}, callback) or fs.readdir('./', {}, callback) the call will fail and the callback will never be called.

The reason: the latest version of grunt-assemble (0.6.3), depends on a very old version of gray-matter (0.4.2, while the latest is 4.0.3). This old gray-matter v0.4.2 depends on old fs-utils 0.4.3, which in turn depends on an old version of graceful-fs (v2.0.3).

This very old version of graceful-fs monkey patches fs.readdir (along with a handful other fs methods). The overriden version does not support fs.readdir( path, options, callback ) format only the older fs.readdir( path, callback ), breaking many recent libraries.

Updating grunt-assemble to use the latest gray-matter will solve the issue, as I believe they are no longer using fs-utils / graceful-fs.

My temporary fix for Gruntfile.js, is to un-monkey patch the affected methods:

  const fs = require('fs');
  const { readdir, close, closeSync, open } = fs;
  .
  .
  .
  grunt.loadNpmTasks('grunt-assemble');
  Object.assign(fs, { readdir, close, closeSync, open });

[jonschlinkert]

Great to know. is it possible for us to patch in a semver-compliant, backwards compatible way?

Also, would you mind if I change the issue name to "graceful-fs breaks system fs.readdir globally"?

[iosart]

Great to know. is it possible for us to patch in a semver-compliant, backwards compatible way?

Also, would you mind if I change the issue name to "graceful-fs breaks system fs.readdir globally"?

Re patching, I believe updating grunt-assemble to use the latest gray-matter (might need some adjustments as gray-matter's API might have changed a bit) should be fully backward compatible externally, AFAIK. So a new patch level version might be fine.

Re issue name, maybe "graceful-fs included by grunt-assemble v0.6.3 breaks system fs.readdir globally"?

[jonschlinkert]

Sounds good on the rename.

Regard the patch, I think as long as unit tests pass, I'm good with that. Do you want to do a PR?

[iosart]

Sounds good on the rename.

Regard the patch, I think as long as unit tests pass, I'm good with that. Do you want to do a PR?

Tried updating to the latest gray-matter, a few tests are failing. I saw some things around differences in new lines, edge cases etc. It all might be fine, but I’m not sure I’m qualified to make that call

[jonschlinkert]

Unfortunately that means we can't do a patch or a minor. Which tests are failing?

[iosart]

3 failures:

1.Reading From Files hbs file with complex yaml data and content:

Input:

---
foo: bar
version: 2
categories:
- pages
tags:
- tests
- examples
- complex
---

<div class="alert alert-info">This is an alert</div>

Expected:

\n\n<div class="alert alert-info">This is an alert</div>\n

Actual:

\n<div class="alert alert-info">This is an alert</div>\n

Actual has only one new line in the beginning vs expected two

2. Reading From Strings yaml string starts with --- no content:

Input:

---\nfoo: bar\n

Expected:

{}

Actual:

{
   "foo": "bar"
}

3. Reading From Strings hbs string with complex yaml data and content:

Input:

---\nfoo: bar\nversion: 2\n---\n\n<div class="alert alert-info">This is an alert</div>\n

Expected:

{
  original: '---\nfoo: bar\nversion: 2\n---\n\n<div class="alert alert-info">This is an alert</div>\n',
  content: '\n\n<div class="alert alert-info">This is an alert</div>\n',
  data: {
    'foo': 'bar',
    'version': 2
  }
}

Actual:

{
  excerpt: '',
  isEmpty: false,
  content: '\n<div class="alert alert-info">This is an alert</div>\n',
  data: { 
    'foo': 'bar', 
    version: 2 
  }
}

new version returns new fields 'excerpt' and 'isEmpty' and doesn't return 'original'. Also, same as first test, returns a single new line vs double expected.