paulmillr / readdirp

Recursive version of fs.readdir with streaming api.
https://paulmillr.com
MIT License
381 stars 52 forks source link

Detection for stat bigint on Windows is not working with Node v14 #170

Closed jmathew closed 3 years ago

jmathew commented 3 years ago

stat.length === 3 in the below code always returns false.

// Use bigint stats if it's windows and stat() supports options (node 10+).
if (process.platform === 'win32' && stat.length === 3) {
  this._stat = path => statMethod(path, { bigint: true });
} else {
  this._stat = statMethod;
}

This appears to be because stat second parameter is optional, thus making the function always return 1.

C:\Users\j>node
Welcome to Node.js v14.9.0.
Type ".help" for more information.
> require('fs').stat.length
1
> require('fs').lstat.length
1
> require('fs').stat.toString()
'function stat(path, options = { bigint: false }, callback) {\r\n' +
  "  if (typeof options === 'function') {\r\n" +
  '    callback = options;\r\n' +
  '    options = {};\r\n' +
  '  }\r\n' +
  '  callback = makeStatsCallback(callback);\r\n' +
  '  path = getValidatedPath(path);\r\n' +
  '  const req = new FSReqCallback(options.bigint);\r\n' +
  '  req.oncomplete = callback;\r\n' +
  '  binding.stat(pathModule.toNamespacedPath(path), options.bigint, req);\r\n' +
  '}'
> ((one, two, three) => {}).length
3
> ((one, two = true, three) => {}).length
1

This behavior appears to be consistent with Mozilla's reading of the spec.

So I think there needs to be a different approach for determining if the stat func supports bigint.

Thoughts? I can take a stab at changing this, but before I do I'd like to confirm I'm not missing anything.

paulmillr commented 3 years ago

that's pretty stupid they've changed the behavior. of course, we'll need a new implementation.