Refactor internal stat utils (#764)

* Refactor internal getStats() util Fixes #762 * Proper promise tests for copy() Our hacky tests don't play well with multiple layers of callback/promise switching * Simplify internal checkParentPaths() * Port improvments to sync methods
jprichardson · Feb 17, 2020 · 075c2d1 · 075c2d1
1 parent ce41762
commit 075c2d1
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 64 deletions.
diff --git a/lib/__tests__/promise.test.js b/lib/__tests__/promise.test.js
@@ -7,7 +7,6 @@ const fs = require('fs')
 const fse = require('..')
 
 const methods = [
-  'copy',
   'emptyDir',
   'ensureFile',
   'ensureDir',

diff --git a/lib/copy/__tests__/copy.test.js b/lib/copy/__tests__/copy.test.js
@@ -72,6 +72,19 @@ describe('fs-extra', () => {
         })
       })
 
+      it('should work with promises', () => {
+        const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_src')
+        const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
+        fs.writeFileSync(fileSrc, crypto.randomBytes(SIZE))
+        const srcMd5 = crypto.createHash('md5').update(fs.readFileSync(fileSrc)).digest('hex')
+        let destMd5 = ''
+
+        return fse.copy(fileSrc, fileDest).then(() => {
+          destMd5 = crypto.createHash('md5').update(fs.readFileSync(fileDest)).digest('hex')
+          assert.strictEqual(srcMd5, destMd5)
+        })
+      })
+
       it('should return an error if src file does not exist', done => {
         const fileSrc = 'we-simply-assume-this-file-does-not-exist.bin'
         const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')

diff --git a/lib/util/stat.js b/lib/util/stat.js
@@ -1,50 +1,29 @@
 'use strict'
 
-const fs = require('graceful-fs')
+const fs = require('../fs')
 const path = require('path')
+const util = require('util')
 const atLeastNode = require('at-least-node')
 
 const nodeSupportsBigInt = atLeastNode('10.5.0')
+const stat = (file) => nodeSupportsBigInt ? fs.stat(file, { bigint: true }) : fs.stat(file)
+const statSync = (file) => nodeSupportsBigInt ? fs.statSync(file, { bigint: true }) : fs.statSync(file)
 
-function getStats (src, dest, cb) {
-  if (nodeSupportsBigInt) {
-    fs.stat(src, { bigint: true }, (err, srcStat) => {
-      if (err) return cb(err)
-      fs.stat(dest, { bigint: true }, (err, destStat) => {
-        if (err) {
-          if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null })
-          return cb(err)
-        }
-        return cb(null, { srcStat, destStat })
-      })
+function getStats (src, dest) {
+  return Promise.all([
+    stat(src),
+    stat(dest).catch(err => {
+      if (err.code === 'ENOENT') return null
+      throw err
     })
-  } else {
-    fs.stat(src, (err, srcStat) => {
-      if (err) return cb(err)
-      fs.stat(dest, (err, destStat) => {
-        if (err) {
-          if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null })
-          return cb(err)
-        }
-        return cb(null, { srcStat, destStat })
-      })
-    })
-  }
+  ]).then(([srcStat, destStat]) => ({ srcStat, destStat }))
 }
 
 function getStatsSync (src, dest) {
-  let srcStat, destStat
-  if (nodeSupportsBigInt) {
-    srcStat = fs.statSync(src, { bigint: true })
-  } else {
-    srcStat = fs.statSync(src)
-  }
+  let destStat
+  const srcStat = statSync(src)
   try {
-    if (nodeSupportsBigInt) {
-      destStat = fs.statSync(dest, { bigint: true })
-    } else {
-      destStat = fs.statSync(dest)
-    }
+    destStat = statSync(dest)
   } catch (err) {
     if (err.code === 'ENOENT') return { srcStat, destStat: null }
     throw err
@@ -53,7 +32,7 @@ function getStatsSync (src, dest) {
 }
 
 function checkPaths (src, dest, funcName, cb) {
-  getStats(src, dest, (err, stats) => {
+  util.callbackify(getStats)(src, dest, (err, stats) => {
     if (err) return cb(err)
     const { srcStat, destStat } = stats
     if (destStat && areIdentical(srcStat, destStat)) {
@@ -85,29 +64,18 @@ function checkParentPaths (src, srcStat, dest, funcName, cb) {
   const srcParent = path.resolve(path.dirname(src))
   const destParent = path.resolve(path.dirname(dest))
   if (destParent === srcParent || destParent === path.parse(destParent).root) return cb()
-  if (nodeSupportsBigInt) {
-    fs.stat(destParent, { bigint: true }, (err, destStat) => {
-      if (err) {
-        if (err.code === 'ENOENT') return cb()
-        return cb(err)
-      }
-      if (areIdentical(srcStat, destStat)) {
-        return cb(new Error(errMsg(src, dest, funcName)))
-      }
-      return checkParentPaths(src, srcStat, destParent, funcName, cb)
-    })
-  } else {
-    fs.stat(destParent, (err, destStat) => {
-      if (err) {
-        if (err.code === 'ENOENT') return cb()
-        return cb(err)
-      }
-      if (areIdentical(srcStat, destStat)) {
-        return cb(new Error(errMsg(src, dest, funcName)))
-      }
-      return checkParentPaths(src, srcStat, destParent, funcName, cb)
-    })
+  const callback = (err, destStat) => {
+    if (err) {
+      if (err.code === 'ENOENT') return cb()
+      return cb(err)
+    }
+    if (areIdentical(srcStat, destStat)) {
+      return cb(new Error(errMsg(src, dest, funcName)))
+    }
+    return checkParentPaths(src, srcStat, destParent, funcName, cb)
   }
+  if (nodeSupportsBigInt) fs.stat(destParent, { bigint: true }, callback)
+  else fs.stat(destParent, callback)
 }
 
 function checkParentPathsSync (src, srcStat, dest, funcName) {
@@ -116,11 +84,7 @@ function checkParentPathsSync (src, srcStat, dest, funcName) {
   if (destParent === srcParent || destParent === path.parse(destParent).root) return
   let destStat
   try {
-    if (nodeSupportsBigInt) {
-      destStat = fs.statSync(destParent, { bigint: true })
-    } else {
-      destStat = fs.statSync(destParent)
-    }
+    destStat = statSync(destParent)
   } catch (err) {
     if (err.code === 'ENOENT') return
     throw err