search

paulmillr / readdirp

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

Stats should respect opts.bigint = false #100

Open diamondap opened 5 years ago

diamondap commented 5 years ago

This issue affects v. 3.1.1 on Windows.

If I call readdirp with opts.bigint = false, it fixes a number of these errors:

TypeError: Cannot mix BigInt and other types, use explicit conversions

However, this line overwrites opts.bigint and forces it to true on Windows:

https://github.com/paulmillr/readdirp/blob/master/index.js#L96

That results in TypeErrors every time my code stats a file on Windows. Seems like opts.bigint should apply consistently. Also, the options section of the README should mention that bigint is now an option, if it was meant to be a publicly exposed option. Maybe it's not, and I'm misusing the package.

paulmillr commented 5 years ago

That was not meant to be exposed as an option, I didn’t want to add complexity. Maybe we should.

diamondap commented 5 years ago

That's fair, since it wasn't actually documented. For now, I've changed all instances of stats.size to Number(stats.size). So at this point, it's not a major issue for me. You might want to consider it if you get complaints about the break in backwards compatibility.

TotallyInformation commented 4 years ago

Using the bigint version of stats can break other things too since bigint doesn't yet seem to be fully supported. For example, moving from v2 to v3 started generating TypeError: Do not know how to serialize a BigInt errors in Node-RED.

This should absolutely be an option please. As a workaround, I've also had to wrap size, uid and gid with Number(...).

paulmillr commented 4 years ago

bigint doesn't yet seem to be fully supported

Just want to clarify: it is supported in node 10 and higher. Node 8 will become unsupported in 3 months, on Jan 1, 2020. Chokidar / readdirp itself support node 8.16+.

paulmillr commented 1 week ago

will do that in v5 which will bump min nodejs to v18, which supports mtimeNs