From 63f8ced3cc898d10df1a7145dc8494f973ebba8a Mon Sep 17 00:00:00 2001 From: SukkaW Date: Wed, 25 Oct 2023 12:41:48 +0800 Subject: [PATCH 1/3] Refactor `move` API to async/await --- lib/move/__tests__/move.test.js | 15 +++--- lib/move/index.js | 2 +- lib/move/move.js | 89 +++++++++++++-------------------- 3 files changed, 44 insertions(+), 62 deletions(-) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index c1cc2a9e..701e7c72 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -1,25 +1,24 @@ 'use strict' -const fs = require('graceful-fs') +const fs = require('../../fs') const os = require('os') const fse = require('../../') const path = require('path') const assert = require('assert') const { differentDevice, ifCrossDeviceEnabled } = require('./cross-device-utils') +const u = require('universalify').fromPromise /* global afterEach, beforeEach, describe, it */ const describeIfWindows = process.platform === 'win32' ? describe : describe.skip function createAsyncErrFn (errCode) { - const fn = function (...args) { + async function fn () { fn.callCount++ - const callback = args.pop() - setTimeout(() => { - const err = new Error() - err.code = errCode - callback(err) - }, 10) + const err = new Error() + err.code = errCode + + return Promise.reject(err) } fn.callCount = 0 return fn diff --git a/lib/move/index.js b/lib/move/index.js index fcee73c4..5a2f1ccd 100644 --- a/lib/move/index.js +++ b/lib/move/index.js @@ -1,6 +1,6 @@ 'use strict' -const u = require('universalify').fromCallback +const u = require('universalify').fromPromise module.exports = { move: u(require('./move')), moveSync: require('./move-sync') diff --git a/lib/move/move.js b/lib/move/move.js index 5c4d74f9..4c225fb8 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -1,76 +1,59 @@ 'use strict' -const fs = require('graceful-fs') +const fs = require('../fs') const path = require('path') -const copy = require('../copy').copy -const remove = require('../remove').remove -const mkdirp = require('../mkdirs').mkdirp -const pathExists = require('../path-exists').pathExists +const { copy } = require('../copy') +const { remove } = require('../remove') +const { mkdirp } = require('../mkdirs') +const { pathExists } = require('../path-exists') const stat = require('../util/stat') -function move (src, dest, opts, cb) { - if (typeof opts === 'function') { - cb = opts - opts = {} - } +async function move(src, dest, opts = {}) { + const overwrite = opts.overwrite || opts.clobber || false - opts = opts || {} + const { srcStat, isChangingCase = false } = await stat.checkPaths(src, dest, 'move', opts) - const overwrite = opts.overwrite || opts.clobber || false + await stat.checkParentPaths(src, srcStat, dest, 'move') - stat.checkPaths(src, dest, 'move', opts, (err, stats) => { - if (err) return cb(err) - const { srcStat, isChangingCase = false } = stats - stat.checkParentPaths(src, srcStat, dest, 'move', err => { - if (err) return cb(err) - if (isParentRoot(dest)) return doRename(src, dest, overwrite, isChangingCase, cb) - mkdirp(path.dirname(dest), err => { - if (err) return cb(err) - return doRename(src, dest, overwrite, isChangingCase, cb) - }) - }) - }) -} + // If the parent of dest is not root, make sure it exists before proceeding + const destParent = path.dirname(dest) + const parsedParentPath = path.parse(destParent) + if (parsedParentPath.root !== destParent) { + await mkdirp(path.dirname(dest)) + } -function isParentRoot (dest) { - const parent = path.dirname(dest) - const parsedPath = path.parse(parent) - return parsedPath.root === parent + return doRename(src, dest, overwrite, isChangingCase) } -function doRename (src, dest, overwrite, isChangingCase, cb) { - if (isChangingCase) return rename(src, dest, overwrite, cb) - if (overwrite) { - return remove(dest, err => { - if (err) return cb(err) - return rename(src, dest, overwrite, cb) - }) +async function doRename(src, dest, overwrite, isChangingCase) { + if (!isChangingCase) { + if (overwrite) { + await remove(dest) + } else if (await pathExists(dest)) { + throw new Error('dest already exists.') + } } - pathExists(dest, (err, destExists) => { - if (err) return cb(err) - if (destExists) return cb(new Error('dest already exists.')) - return rename(src, dest, overwrite, cb) - }) -} -function rename (src, dest, overwrite, cb) { - fs.rename(src, dest, err => { - if (!err) return cb() - if (err.code !== 'EXDEV') return cb(err) - return moveAcrossDevice(src, dest, overwrite, cb) - }) + try { + // Try w/ rename first, and try copy + remove if EXDEV + await fs.rename(src, dest) + } catch (err) { + if (err.code !== 'EXDEV') { + throw err + } + await moveAcrossDevice(src, dest, overwrite) + } } -function moveAcrossDevice (src, dest, overwrite, cb) { +async function moveAcrossDevice(src, dest, overwrite) { const opts = { overwrite, errorOnExist: true, preserveTimestamps: true } - copy(src, dest, opts, err => { - if (err) return cb(err) - return remove(src, cb) - }) + + await copy(src, dest, opts) + return remove(src) } module.exports = move From 5f28c734ecee5068c4bf8c7844f41091c589106e Mon Sep 17 00:00:00 2001 From: SukkaW Date: Wed, 25 Oct 2023 12:55:40 +0800 Subject: [PATCH 2/3] Make `standard` happy --- lib/move/__tests__/move.test.js | 1 - lib/move/move.js | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/move/__tests__/move.test.js b/lib/move/__tests__/move.test.js index 701e7c72..fd905472 100644 --- a/lib/move/__tests__/move.test.js +++ b/lib/move/__tests__/move.test.js @@ -6,7 +6,6 @@ const fse = require('../../') const path = require('path') const assert = require('assert') const { differentDevice, ifCrossDeviceEnabled } = require('./cross-device-utils') -const u = require('universalify').fromPromise /* global afterEach, beforeEach, describe, it */ diff --git a/lib/move/move.js b/lib/move/move.js index 4c225fb8..0a1f33e8 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -8,7 +8,7 @@ const { mkdirp } = require('../mkdirs') const { pathExists } = require('../path-exists') const stat = require('../util/stat') -async function move(src, dest, opts = {}) { +async function move (src, dest, opts = {}) { const overwrite = opts.overwrite || opts.clobber || false const { srcStat, isChangingCase = false } = await stat.checkPaths(src, dest, 'move', opts) @@ -25,7 +25,7 @@ async function move(src, dest, opts = {}) { return doRename(src, dest, overwrite, isChangingCase) } -async function doRename(src, dest, overwrite, isChangingCase) { +async function doRename (src, dest, overwrite, isChangingCase) { if (!isChangingCase) { if (overwrite) { await remove(dest) @@ -45,7 +45,7 @@ async function doRename(src, dest, overwrite, isChangingCase) { } } -async function moveAcrossDevice(src, dest, overwrite) { +async function moveAcrossDevice (src, dest, overwrite) { const opts = { overwrite, errorOnExist: true, From 7ab00d58bf1f018aaa2f3490e52c5fa401c7c449 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Thu, 26 Oct 2023 11:48:11 +0800 Subject: [PATCH 3/3] Apply code review suggestions from @RyanZim --- lib/move/move.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/move/move.js b/lib/move/move.js index 0a1f33e8..992bd0fc 100644 --- a/lib/move/move.js +++ b/lib/move/move.js @@ -19,7 +19,7 @@ async function move (src, dest, opts = {}) { const destParent = path.dirname(dest) const parsedParentPath = path.parse(destParent) if (parsedParentPath.root !== destParent) { - await mkdirp(path.dirname(dest)) + await mkdirp(destParent) } return doRename(src, dest, overwrite, isChangingCase)