Skip to content

Commit

Permalink
default to manual on Windows
Browse files Browse the repository at this point in the history
Looking at benchmarks, it seems to be faster to use this impl on Windows
than Node's built-in version, and there's fallback to move-remove, which
makes it more resilient as well.
  • Loading branch information
isaacs committed Jan 13, 2023
1 parent 3b6b098 commit d9b722e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 10 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Install with `npm install rimraf`, or just drop rimraf.js somewhere.
- The function returns a `Promise` instead of taking a callback.
- Built-in glob support removed.
- Functions take arrays of paths, as well as a single path.
- Native implementation used by default when available.
- Native implementation used by default when available, except on
Windows, where this implementation is faster and more reliable.
- New implementation on Windows, falling back to "move then
remove" strategy when exponential backoff for `EBUSY` fails to
resolve the situation.
Expand Down
9 changes: 7 additions & 2 deletions src/use-native.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
const version = process.env.__TESTING_RIMRAF_NODE_VERSION__ || process.version
const versArr = version.replace(/^v/, '').split('.')
const hasNative = +versArr[0] > 14 || (+versArr[0] === 14 && +versArr[1] >= 14)
export const useNative = !hasNative ? () => false : () => true
export const useNativeSync = !hasNative ? () => false : () => true
// we do NOT use native by default on Windows, because Node's native
// rm implementation is less advanced. Change this code if that changes.
import platform from './platform'
export const useNative =
!hasNative || platform === 'win32' ? () => false : () => true
export const useNativeSync =
!hasNative || platform === 'win32' ? () => false : () => true
33 changes: 26 additions & 7 deletions test/use-native.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,44 @@ if (/^v([0-8]\.|1[0-3]\.|14\.[0-9]\.|14\.1[1-3]\.)/.test(process.version)) {
const t = require('tap')
const { useNative, useNativeSync } = require('../dist/cjs/src/use-native.js')

if (!process.env.__TESTING_RIMRAF_NODE_VERSION__) {
if (!process.env.__TESTING_RIMRAF_EXPECT_USE_NATIVE__) {
t.spawn(process.execPath, [__filename], {
env: {
...process.env,
__TESTING_RIMRAF_NODE_VERSION__: 'v14.13.12',
__TESTING_RIMRAF_PLATFORM__: 'darwin',
__TESTING_RIMRAF_NODE_VERSION__: 'v18.0.0',
__TESTING_RIMRAF_EXPECT_USE_NATIVE__: '1',
},
})

t.spawn(process.execPath, [__filename], {
env: {
...process.env,
__TESTING_RIMRAF_PLATFORM__: 'win32',
__TESTING_RIMRAF_NODE_VERSION__: 'v18.0.0',
__TESTING_RIMRAF_EXPECT_USE_NATIVE__: '0',
},
})

t.spawn(process.execPath, [__filename], {
env: {
...process.env,
__TESTING_RIMRAF_NODE_VERSION__: 'v8.9.10',
__TESTING_RIMRAF_PLATFORM__: 'darwin',
__TESTING_RIMRAF_EXPECT_USE_NATIVE__: '0',
},
})

// this one has the native impl
t.equal(useNative(), true)
t.equal(useNativeSync(), true)
t.spawn(process.execPath, [__filename], {
env: {
...process.env,
__TESTING_RIMRAF_NODE_VERSION__: 'v14.13.12',
__TESTING_RIMRAF_PLATFORM__: 'darwin',
__TESTING_RIMRAF_EXPECT_USE_NATIVE__: '0',
},
})
} else {
t.equal(useNative(), false)
t.equal(useNativeSync(), false)
const expect = process.env.__TESTING_RIMRAF_EXPECT_USE_NATIVE__ === '1'
t.equal(useNative(), expect)
t.equal(useNativeSync(), expect)
}

0 comments on commit d9b722e

Please sign in to comment.