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

fix: Resolve dependency issues on Node.js 20 #62

Merged
merged 2 commits into from
Sep 15, 2023
Merged

Conversation

jduncanator
Copy link
Owner

@jduncanator jduncanator commented Sep 13, 2023

This bumps dependencies to resolve installation issues with Node 20. It also drops the native module and makes use of fs.statfs where available (Node.js >=18).

Ideally we wouldn't require node-gyp, or the native module at all when installing under supported engines, but there doesn't seem to be an elegant way to achieve that, and I don't want to drop support for Node <18 (yet).

This supersedes #61, and should resolve #60 and #59. @francois-spectre and @radjybaba, testing would be appreciated to confirm this fixes your issues!

Resisting the urge to type const, use object destructuring, and lambdas whilst making this change was very hard... it hurts my eyes (and my soul).

We drop the requirement for a native module, and make use of fs.statfs where available (Node.js >=18)
Rewrites the checkSync implementation under Node 18+
@jduncanator
Copy link
Owner Author

jduncanator commented Sep 13, 2023

I've verified these changes work all the way back to Node 4.9.1 (on Linux):

root@c66171b3c386:~/node-diskusage# node
> process.version
'v4.9.1'
> require('.').checkSync('/')
{ available: [...],
  free: [...],
  total: [...] }
>

Node 6.17.1 (on Linux):

root@734831f2b9b3:~/node-diskusage# node
> process.version
'v6.17.1'
> require('.').checkSync('/')
{ available: [...],
  free: [...],
  total: [...] }
>

As well as in Node 20.6.1 (on Windows):

> process.version
'v20.6.1'
> require('.').checkSync('C:')
{ available: [...], free: [...], total: [...] }
>

I'm unable to get Node 0.12 to work, I struggle to even install node-gyp as a dep, not sure if it's always been that way or something I've broken with this change, but I don't mind dropping support for 0.12 😅

@jduncanator
Copy link
Owner Author

jduncanator commented Sep 13, 2023

Turns out the Docker image I was using was broken. Using node:0.12, I'm able to build and verify everything still works all the way back to Node 0.12.18:

root@6aa0edf4a485:~/node-diskusage# node
> process.version
'v0.12.18'
> require('./').checkSync('/')
{ available: [...],
  free: [...],
  total: [...] }

Unless anyone has objections or issues, I'll merge this in the morning (~9 hours from now)!

@jduncanator jduncanator merged commit 0f89ad0 into master Sep 15, 2023
@jduncanator jduncanator deleted the fix/node20 branch September 15, 2023 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install is broken with node 20
1 participant