From d9b722e84378adf43319f8bb9a0f3cf912cee0af Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 12 Jan 2023 16:53:29 -0800 Subject: [PATCH] default to manual on Windows 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. --- README.md | 3 ++- src/use-native.ts | 9 +++++++-- test/use-native.js | 33 ++++++++++++++++++++++++++------- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 0418112c..66a01831 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/src/use-native.ts b/src/use-native.ts index 92533d6a..d60ed036 100644 --- a/src/use-native.ts +++ b/src/use-native.ts @@ -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 diff --git a/test/use-native.js b/test/use-native.js index 0ca8e83d..943c1bf7 100644 --- a/test/use-native.js +++ b/test/use-native.js @@ -8,11 +8,22 @@ 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', }, }) @@ -20,13 +31,21 @@ if (!process.env.__TESTING_RIMRAF_NODE_VERSION__) { 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) }