-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
chore: update dependencies and require node 18 #161
Conversation
unlink: gracefulFs.unlink, | ||
unlinkSync: gracefulFs.unlinkSync, | ||
chmod: gracefulFs.chmod, | ||
chmodSync: gracefulFs.chmodSync, | ||
stat: gracefulFs.stat, | ||
statSync: gracefulFs.statSync, | ||
lstat: gracefulFs.lstat, | ||
lstatSync: gracefulFs.lstatSync, | ||
rmdir: gracefulFs.rmdir, | ||
rmdirSync: gracefulFs.rmdirSync, | ||
readdir: gracefulFs.readdir, | ||
readdirSync: gracefulFs.readdirSync, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the support for a custom fs
layer in rimraf
have long been removed - hence cleaning this up and removing the dependency on graceful-fs
in the process.
6cbaa48
to
1670c1f
Compare
1670c1f
to
6b52365
Compare
@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 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 ... |
Sorry, missed this one. I'm happy to merge it when tests are passing. |
@pmmmwh I primarily use Windows, let me know if you would like some help with this |
f6fb083
to
6223872
Compare
I did some testing on a Windows VM - it seems like this is caused by a bug in From my testing, the issue seemed to be related to how |
Created isaacs/rimraf#314 - unfortunately this is targeting @sindresorhus don't really know what's the way forward here - I could change the test slightly to ignore |
index.js
Outdated
import pMap from 'p-map'; | ||
|
||
const rimrafP = promisify(rimraf); | ||
import {rimraf} from 'rimraf'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think this is needed anymore, right? This has been available since node 14.14
fs.rmSync(dir, { recursive: true, force: true });
Blocked on isaacs/rimraf#303 |
Fixes #162 |
Co-authored-by: Mark Wiemer <7833360+mark-wiemer@users.noreply.github.com>
Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Since we've moved to native fs.rm - I think we can safely drop the |
Already done |
This PR updates all the dependencies consumed by
del
- particularly moving fromrimraf@3
torimraf@5
which upgrades toglob@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