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

stats: true outputs different data shape #246

Closed
TheJaredWilcurt opened this issue Jan 1, 2020 · 3 comments
Closed

stats: true outputs different data shape #246

TheJaredWilcurt opened this issue Jan 1, 2020 · 3 comments
Assignees

Comments

@TheJaredWilcurt
Copy link

Environment

  • OS Version: …
  • Node.js Version: …

Actual behavior

const glob = require('fast-glob');
glob.sync(['*/**'], { ignore: ['node_modules'], stats: false });
// ['src/index.js']
glob.sync(['*/**'], { ignore: ['node_modules'], stats: true });
/*
[ { name: 'src/index.js',
    path: 'src/index.js',
    dirent:
     DirentFromStats {
       name: 'src/index.js',
       isBlockDevice: [Function: bound ],
       isCharacterDevice: [Function: bound ],
       isDirectory: [Function: bound ],
       isFIFO: [Function: bound ],
       isFile: [Function: bound ],
       isSocket: [Function: bound ],
       isSymbolicLink: [Function: bound ] },
    stats:
     Stats {
       dev: 609818264,
       mode: 33206,
       nlink: 1,
       uid: 0,
       gid: 0,
       rdev: 0,
       blksize: undefined,
       ino: 14636698789945968,
       size: 7642,
       blocks: undefined,
       atimeMs: 1576352894838.0518,
       mtimeMs: 1577798007537,
       ctimeMs: 1577798007537,
       birthtimeMs: 1575041614453.75,
       atime: 2019-12-14T19:48:14.838Z,
       mtime: 2019-12-31T13:13:27.537Z,
       ctime: 2019-12-31T13:13:27.537Z,
       birthtime: 2019-11-29T15:33:34.454Z } } ]
*/

Expected behavior

The documentation states that the difference between stats: true/false is merely performance and the speed increase comes from newer Node versions.

I was having issues testing this, because mock-fs got confused when stats: false was used (or stats was omitted). I found a GitHub issue saying to fix this just run the slower version when running your tests:

const isJest = typeof(process.env.JEST_WORKER_ID) === 'string';
glob.sync(['*/**'], { ignore: ['node_modules'], stats: isJest });

But this means my tests and my source code are expecting different data shapes.

My only course of action now is to always use the slow option to ensure it works everywhere.

It seems as though stats has two meanings. A boolean to double or half your performance. And a boolean to decide if you only need file paths, or details about each file. These concepts should not be linked like this.

@TheJaredWilcurt
Copy link
Author

For anyone else finding their way here from a google search, here is the TLDR:

const fs = require('fs-extra');
const glob = require('fast-glob');
const path = require('path');

function copyFiles (filePatterns, excludes, dist) {
  /*
   Mock-fs in our unit tests, and versions of Node below 10.10, require stats to be true.
   This will return an array of detailed objects about each file. stats: false is roughly
   twice as fast and returns only an array of strings of the paths. This would be preferred
   in the future for the performance increase, but for now, we have to live with this for
   compatibility.
  */
  const weAreUsingMockFsOrNodeUnder10dot10 = true;
  const filesToCopy = glob.sync(filePatterns, {
    ignore: excludes,
    stats: weAreUsingMockFsOrNodeUnder10dot10
  });

  filesToCopy.forEach(function (file) {
    try {
      fs.copySync(file.path, path.join(dist, file.path));
    } catch (err) {
      console.log('Error copying file.');
      console.log(file);
      console.log(err);
    }
  });
}

@mrmlnc
Copy link
Owner

mrmlnc commented Jan 3, 2020

I'm sorry, but I don't see how I can help you.

The fast-glob package has only one option to control the output format: objectMode. The stats option enables the objectMode definitely. But you can use the objectMode without stats option.

Most likely the mock-fs package does not support the readdir method with withFileTypes option (documentation for modern mode — Node.js 10.10+). And, yeap, it is true — tschaub/mock-fs#272. In this case the objectMode option cannot help you.

@mrmlnc mrmlnc self-assigned this Jan 3, 2020
@mrmlnc mrmlnc closed this as completed Jan 3, 2020
@mrmlnc
Copy link
Owner

mrmlnc commented Jan 3, 2020

Thanks for the feedback!

Closed in favor of tschaub/mock-fs#272 and possible solution — tschaub/mock-fs#287.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants