Major refactor (#128)

* BREAKING: Drop Node 6 & 8 * Upgrade universalify * Refactor writeFileSync() * BREAKING: Refactor writeFile(); do not allow passing null options * Refactor readFileSync() * BREAKING: Refactor readFile(); do not allow passing null options * Refactor internal functions and move them to a seperate file
jprichardson · Feb 24, 2020 · e3d86e0 · e3d86e0
1 parent 7b53118
commit e3d86e0
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 145 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,8 +1,6 @@
 sudo: false
 language: node_js
 node_js:
-  - "6"
-  - "8"
   - "10"
   - "12"
 matrix:

diff --git a/appveyor.yml b/appveyor.yml
@@ -2,8 +2,6 @@
 environment:
   matrix:
     # node.js
-    - nodejs_version: "6"
-    - nodejs_version: "8"
     - nodejs_version: "10"
     - nodejs_version: "12"
 

diff --git a/index.js b/index.js
@@ -5,60 +5,46 @@ try {
   _fs = require('fs')
 }
 const universalify = require('universalify')
+const { stringify, stripBom } = require('./utils')
 
-function readFileWithCallback (file, options, callback) {
-  if (callback == null) {
-    callback = options
-    options = {}
-  }
-
+async function _readFile (file, options = {}) {
   if (typeof options === 'string') {
     options = { encoding: options }
   }
 
-  options = options || {}
   const fs = options.fs || _fs
 
-  let shouldThrow = true
-  if ('throws' in options) {
-    shouldThrow = options.throws
-  }
+  const shouldThrow = 'throws' in options ? options.throws : true
+
+  let data = await universalify.fromCallback(fs.readFile)(file, options)
 
-  fs.readFile(file, options, (err, data) => {
-    if (err) return callback(err)
-
-    data = stripBom(data)
-
-    let obj
-    try {
-      obj = JSON.parse(data, options ? options.reviver : null)
-    } catch (err2) {
-      if (shouldThrow) {
-        err2.message = `${file}: ${err2.message}`
-        return callback(err2)
-      } else {
-        return callback(null, null)
-      }
+  data = stripBom(data)
+
+  let obj
+  try {
+    obj = JSON.parse(data, options ? options.reviver : null)
+  } catch (err) {
+    if (shouldThrow) {
+      err.message = `${file}: ${err.message}`
+      throw err
+    } else {
+      return null
     }
+  }
 
-    callback(null, obj)
-  })
+  return obj
 }
 
-const readFile = universalify.fromCallback(readFileWithCallback)
+const readFile = universalify.fromPromise(_readFile)
 
-function readFileSync (file, options) {
-  options = options || {}
+function readFileSync (file, options = {}) {
   if (typeof options === 'string') {
     options = { encoding: options }
   }
 
   const fs = options.fs || _fs
 
-  let shouldThrow = true
-  if ('throws' in options) {
-    shouldThrow = options.throws
-  }
+  const shouldThrow = 'throws' in options ? options.throws : true
 
   try {
     let content = fs.readFileSync(file, options)
@@ -74,59 +60,24 @@ function readFileSync (file, options) {
   }
 }
 
-function stringify (obj, options) {
-  let spaces
-  let EOL = '\n'
-  if (typeof options === 'object' && options !== null) {
-    if (options.spaces) {
-      spaces = options.spaces
-    }
-    if (options.EOL) {
-      EOL = options.EOL
-    }
-  }
-
-  const str = JSON.stringify(obj, options ? options.replacer : null, spaces)
-
-  return str.replace(/\n/g, EOL) + EOL
-}
-
-function writeFileWithCallback (file, obj, options, callback) {
-  if (callback == null) {
-    callback = options
-    options = {}
-  }
-  options = options || {}
+async function _writeFile (file, obj, options = {}) {
   const fs = options.fs || _fs
 
-  let str = ''
-  try {
-    str = stringify(obj, options)
-  } catch (err) {
-    return callback(err, null)
-  }
+  const str = stringify(obj, options)
 
-  fs.writeFile(file, str, options, callback)
+  await universalify.fromCallback(fs.writeFile)(file, str, options)
 }
 
-const writeFile = universalify.fromCallback(writeFileWithCallback)
+const writeFile = universalify.fromPromise(_writeFile)
 
-function writeFileSync (file, obj, options) {
-  options = options || {}
+function writeFileSync (file, obj, options = {}) {
   const fs = options.fs || _fs
 
   const str = stringify(obj, options)
   // not sure if fs.writeFileSync returns anything, but just in case
   return fs.writeFileSync(file, str, options)
 }
 
-function stripBom (content) {
-  // we do this because JSON.parse would convert it to a utf8 string if encoding wasn't specified
-  if (Buffer.isBuffer(content)) content = content.toString('utf8')
-  content = content.replace(/^\uFEFF/, '')
-  return content
-}
-
 const jsonfile = {
   readFile,
   readFileSync,

diff --git a/package.json b/package.json
@@ -17,7 +17,7 @@
   "author": "JP Richardson <[email protected]>",
   "license": "MIT",
   "dependencies": {
-    "universalify": "^0.1.2"
+    "universalify": "^0.2.0"
   },
   "optionalDependencies": {
     "graceful-fs": "^4.1.6"
@@ -29,7 +29,8 @@
   },
   "main": "index.js",
   "files": [
-    "index.js"
+    "index.js",
+    "utils.js"
   ],
   "scripts": {
     "lint": "standard",

diff --git a/test/read-file.test.js b/test/read-file.test.js
@@ -206,44 +206,6 @@ describe('+ readFile()', () => {
     })
   })
 
-  describe('> when passing null as options and callback', () => {
-    it('should not throw an error', (done) => {
-      const file = path.join(TEST_DIR, 'somefile.json')
-
-      const obj = {
-        name: 'jp'
-      }
-      fs.writeFileSync(file, JSON.stringify(obj))
-
-      jf.readFile(file, null, (err) => {
-        assert.ifError(err)
-        assert.strictEqual(obj.name, 'jp')
-        done()
-      })
-    })
-  })
-
-  describe('> when passing null as options and expecting a promise', () => {
-    it('should resolve the promise', (done) => {
-      const file = path.join(TEST_DIR, 'somefile.json')
-
-      const obj = {
-        name: 'jp'
-      }
-      fs.writeFileSync(file, JSON.stringify(obj))
-
-      jf.readFile(file, null)
-        .then(data => {
-          assert.strictEqual(data.name, obj.name)
-          done()
-        })
-        .catch(err => {
-          assert.ifError(err)
-          done()
-        })
-    })
-  })
-
   describe('> when passing encoding string as option', () => {
     let file, obj
 

diff --git a/test/write-file.test.js b/test/write-file.test.js
@@ -107,32 +107,6 @@ describe('+ writeFile()', () => {
     })
   })
 
-  describe('> when passing null as options and callback', () => {
-    it('should not throw an error', (done) => {
-      const file = path.join(TEST_DIR, 'somefile.json')
-      const obj = { name: 'jp' }
-      jf.writeFile(file, obj, null, (err) => {
-        assert.ifError(err)
-        done()
-      })
-    })
-  })
-
-  describe('> when passing null as options and No callback', () => {
-    it('should not throw an error', (done) => {
-      const file = path.join(TEST_DIR, 'somefile.json')
-      const obj = { name: 'jp' }
-      jf.writeFile(file, obj, null)
-        .then(res => {
-          done()
-        })
-        .catch(err => {
-          assert.ifError(err)
-          done()
-        })
-    })
-  })
-
   describe('> when spaces passed as an option', () => {
     let file, obj
     beforeEach((done) => {

diff --git a/utils.js b/utils.js
@@ -0,0 +1,15 @@
+function stringify (obj, options = {}) {
+  let EOL = options.EOL || '\n'
+
+  const str = JSON.stringify(obj, options ? options.replacer : null, options.spaces)
+
+  return str.replace(/\n/g, EOL) + EOL
+}
+
+function stripBom (content) {
+  // we do this because JSON.parse would convert it to a utf8 string if encoding wasn't specified
+  if (Buffer.isBuffer(content)) content = content.toString('utf8')
+  return content.replace(/^\uFEFF/, '')
+}
+
+module.exports = { stringify, stripBom }