search

sindresorhus / del

Delete files and directories
MIT License
1.33k stars 66 forks source link

chore: update dependencies and require node 18 #161

Closed pmmmwh closed 1 month ago

pmmmwh commented 5 months ago

This PR updates all the dependencies consumed by del - particularly moving from rimraf@3 to rimraf@5 which upgrades to glob@9 (unused code path).

The main intention is to resolve as much deprecation warnings from npm as possible; but I also bumped minimum supported Node.js version to 18 since that's the oldest version in maintenance, since I presume the upgrade of dependencies would at least warrant a minor and might as well bundle "breaking" changes altogether.

Fixes #162

kibertoad commented 5 months ago

@sindresorhus If compatibility with older Node versions is important for you at this point, I can prepare an alternative PR that only updates the rimraf, which is the most impactful part here.

pmmmwh commented 5 months ago

@sindresorhus If compatibility with older Node versions is important for you at this point, I can prepare an alternative PR that only updates the rimraf, which is the most impactful part here.

While looking at other deps used by del and other packages Sindre maintains my hunch is that this wouldn't be the case, but I can also update this PR to not do that (it's done such that we can bump other dependencies too).

Also - tests are failing on Windows, I'm not entirely sure why and don't have a Windows machine lying around to test - will try to dig into it but help would be appreciated ...

sindresorhus commented 5 months ago

Sorry, missed this one. I'm happy to merge it when tests are passing.

kibertoad commented 5 months ago

@pmmmwh I primarily use Windows, let me know if you would like some help with this

pmmmwh commented 4 months ago

I did some testing on a Windows VM - it seems like this is caused by a bug in rimraf which causes intermittent EPERM issues due to async timing. The test that fails here suffers from this due to the amount of tries.

From my testing, the issue seemed to be related to how rimraf doesn't use fs/promises but rather promisify fs themselves - will try to raise a PR there to see if it will get resolved.

pmmmwh commented 4 months ago

Created isaacs/rimraf#314 - unfortunately this is targeting v6 which only supports Node.js v20+ - we'll have to see if back porting the fix is acceptable for what we're doing here.

@sindresorhus don't really know what's the way forward here - I could change the test slightly to ignore EPERM on Windows (I think it's originally here for macOS issues), but not sure if that's preferable ...

mark-wiemer commented 1 month ago

Blocked on https://github.com/isaacs/rimraf/issues/303

mark-wiemer commented 1 month ago

Fixes #162

pmmmwh commented 1 month ago

Since we've moved to native fs.rm - I think we can safely drop the rimraf dependency too?

sindresorhus commented 1 month ago

Already done