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

Bigint support #844

Closed
fabiospampinato opened this issue May 5, 2019 · 5 comments
Closed

Bigint support #844

fabiospampinato opened this issue May 5, 2019 · 5 comments

Comments

@fabiospampinato
Copy link

fabiospampinato commented May 5, 2019

I'm currently using fs.Stats.ino numbers to detect renames, but this is especially flawed on Windows (nodejs/node#12115) where multiple files can get assigned the same ino number because they overflow the maximum storable number in JS.

The solution would be to tell fs.stat to return bigints instead, which don't have this issue.

I could query fs.stat with the { bigint: true } option myself, but I'd rather reuse the stats object provided by chokidar, for performance.

I see no option for enabling bigints in chokidar, can it be added?

@paulmillr
Copy link
Owner

Should be fixed in new chokidar release.

@fabiospampinato
Copy link
Author

@paulirish has an option been added for this already? And can you say roundabout when the new release will be published?

@paulmillr
Copy link
Owner

It'll use bigints on windows automatically for nodejs 10+. Try it out later this week.

@fabiospampinato
Copy link
Author

@paulirish I'm not sure this implicit behavior is necessarily what we want, wouldn't it be better to enable bigints on all platforms instead? The fact that currently those regular ino numbers overflow on Windows may change with a macOS update or something, maybe it happens already under macOS as well but it's more rare.

@paulmillr
Copy link
Owner

I think they've added the option because of windows FS. If we'll receive additional bug reports, we'll add it. I think it brings some overhead, but not sure.

BTW readdirp 3.0.3 is out — so do npm update and try it; it'll use bigints.

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

2 participants