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 should respect opts.bigint = false #100

Open
diamondap opened this issue Jul 11, 2019 · 5 comments
Open

Stats should respect opts.bigint = false #100

diamondap opened this issue Jul 11, 2019 · 5 comments

Comments

@diamondap
Copy link

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
Copy link
Owner

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

@diamondap
Copy link
Author

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
Copy link

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
Copy link
Owner

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
Copy link
Owner

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

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

No branches or pull requests

3 participants