Simplify options handling

Author: ehmicky

.eslintrc.js

@@ -8,7 +8,6 @@ module.exports = {
     'max-nested-callbacks': 0,
     'max-statements': 0,
     'no-await-in-loop': 0,
-    'no-param-reassign': 0,
     'fp/no-class': 0,
     'fp/no-let': 0,
     'fp/no-loops': 0,

src/deploy/hash_files.js

@@ -3,31 +3,27 @@ const { promisify } = require('util')
 const walker = require('folder-walker')
 const pump = promisify(require('pump'))
 
-const { DEFAULT_CONCURRENT_HASH } = require('./constants')
 const { hasherCtor, manifestCollectorCtor, fileFilterCtor, fileNormalizerCtor } = require('./hasher_segments')
 
-const hashFiles = async (dir, configPath, opts) => {
-  opts = {
-    concurrentHash: DEFAULT_CONCURRENT_HASH,
-    assetType: 'file',
-    statusCb: () => {},
-    ...opts,
-  }
-
-  if (!opts.filter) throw new Error('Missing filter function option')
-  const fileStream = walker([configPath, dir], { filter: opts.filter })
-  const filter = fileFilterCtor()
-  const hasher = hasherCtor(opts)
-  const fileNormalizer = fileNormalizerCtor(opts)
+const hashFiles = async (
+  dir,
+  configPath,
+  { concurrentHash, hashAlgorithm = 'sha1', assetType = 'file', statusCb, filter },
+) => {
+  if (!filter) throw new Error('Missing filter function option')
+  const fileStream = walker([configPath, dir], { filter })
+  const fileFilter = fileFilterCtor()
+  const hasher = hasherCtor({ concurrentHash, hashAlgorithm })
+  const fileNormalizer = fileNormalizerCtor({ assetType })
 
   // Written to by manifestCollector
   // normalizedPath: hash (wanted by deploy API)
   const files = {}
   // hash: [fileObj, fileObj, fileObj]
   const filesShaMap = {}
-  const manifestCollector = manifestCollectorCtor(files, filesShaMap, opts)
+  const manifestCollector = manifestCollectorCtor(files, filesShaMap, { statusCb, assetType })
 
-  await pump(fileStream, filter, hasher, fileNormalizer, manifestCollector)
+  await pump(fileStream, fileFilter, hasher, fileNormalizer, manifestCollector)
 
   return { files, filesShaMap }
 }

src/deploy/hash_files.test.js

@@ -2,12 +2,15 @@ const path = require('path')
 
 const test = require('ava')
 
+const { DEFAULT_CONCURRENT_HASH } = require('./constants')
 const hashFiles = require('./hash_files')
 const { defaultFilter } = require('./util')
 
 test('hashes files in a folder', async (t) => {
   const { files, filesShaMap } = await hashFiles(__dirname, path.resolve(__dirname, '../../fixtures/netlify.toml'), {
     filter: defaultFilter,
+    concurrentHash: DEFAULT_CONCURRENT_HASH,
+    statusCb() {},
   })
   t.truthy(files['netlify.toml'], 'includes the netlify.toml file')
   Object.keys(files).forEach((filePath) => {

src/deploy/hash_fns.js

@@ -5,28 +5,19 @@ const zipIt = require('@netlify/zip-it-and-ship-it')
 const fromArray = require('from2-array')
 const pump = promisify(require('pump'))
 
-const { DEFAULT_CONCURRENT_HASH } = require('./constants')
 const { hasherCtor, manifestCollectorCtor } = require('./hasher_segments')
 
-const hashFns = async (dir, opts) => {
-  opts = {
-    concurrentHash: DEFAULT_CONCURRENT_HASH,
-    assetType: 'function',
-    hashAlgorithm: 'sha256',
-    // tmpDir,
-    statusCb: () => {},
-    ...opts,
-  }
+const hashFns = async (dir, { tmpDir, concurrentHash, hashAlgorithm = 'sha256', assetType = 'function', statusCb }) => {
   // early out if the functions dir is omitted
   if (!dir) return { functions: {}, shaMap: {} }
-  if (!opts.tmpDir) throw new Error('Missing tmpDir directory for zipping files')
+  if (!tmpDir) throw new Error('Missing tmpDir directory for zipping files')
 
-  const functionZips = await zipIt.zipFunctions(dir, opts.tmpDir)
+  const functionZips = await zipIt.zipFunctions(dir, tmpDir)
 
   const fileObjs = functionZips.map(({ path: functionPath, runtime }) => ({
     filepath: functionPath,
-    root: opts.tmpDir,
-    relname: path.relative(opts.tmpDir, functionPath),
+    root: tmpDir,
+    relname: path.relative(tmpDir, functionPath),
     basename: path.basename(functionPath),
     extname: path.extname(functionPath),
     type: 'file',
@@ -37,14 +28,14 @@ const hashFns = async (dir, opts) => {
 
   const functionStream = fromArray.obj(fileObjs)
 
-  const hasher = hasherCtor(opts)
+  const hasher = hasherCtor({ concurrentHash, hashAlgorithm })
 
   // Written to by manifestCollector
   // normalizedPath: hash (wanted by deploy API)
   const functions = {}
   // hash: [fileObj, fileObj, fileObj]
   const fnShaMap = {}
-  const manifestCollector = manifestCollectorCtor(functions, fnShaMap, opts)
+  const manifestCollector = manifestCollectorCtor(functions, fnShaMap, { statusCb, assetType })
 
   await pump(functionStream, hasher, manifestCollector)
 

src/deploy/hash_fns.test.js

@@ -1,11 +1,15 @@
 const test = require('ava')
 const tempy = require('tempy')
 
+const { DEFAULT_CONCURRENT_HASH } = require('./constants')
 const hashFns = require('./hash_fns')
-const { defaultFilter } = require('./util')
 
 test('hashes files in a folder', async (t) => {
-  const { functions, fnShaMap } = await hashFns(__dirname, { filter: defaultFilter, tmpDir: tempy.directory() })
+  const { functions, fnShaMap } = await hashFns(__dirname, {
+    tmpDir: tempy.directory(),
+    concurrentHash: DEFAULT_CONCURRENT_HASH,
+    statusCb() {},
+  })
 
   Object.keys(functions).forEach((path) => {
     t.truthy(path, 'each file has a path')

src/deploy/hasher_segments.js

@@ -7,7 +7,7 @@ const map = require('through2-map').obj
 const { normalizePath } = require('./util')
 
 // a parallel transform stream segment ctor that hashes fileObj's created by folder-walker
-const hasherCtor = ({ concurrentHash, hashAlgorithm = 'sha1' }) => {
+const hasherCtor = ({ concurrentHash, hashAlgorithm }) => {
   const hashaOpts = { algorithm: hashAlgorithm }
   if (!concurrentHash) throw new Error('Missing required opts')
   return transform(concurrentHash, { objectMode: true }, async (fileObj, cb) => {
@@ -22,13 +22,14 @@ const hasherCtor = ({ concurrentHash, hashAlgorithm = 'sha1' }) => {
 }
 
 // Inject normalized file names into normalizedPath and assetType
-const fileNormalizerCtor = ({ assetType = 'file' }) =>
+const fileNormalizerCtor = ({ assetType }) =>
   map((fileObj) => ({ ...fileObj, assetType, normalizedPath: normalizePath(fileObj.relname) }))
 
 // A writable stream segment ctor that normalizes file paths, and writes shaMap's
 const manifestCollectorCtor = (filesObj, shaMap, { statusCb, assetType }) => {
   if (!statusCb || !assetType) throw new Error('Missing required options')
   return objWriter((fileObj, _, cb) => {
+    // eslint-disable-next-line no-param-reassign
     filesObj[fileObj.normalizedPath] = fileObj.hash
 
     // We map a hash to multiple fileObj's because the same file
@@ -37,6 +38,7 @@ const manifestCollectorCtor = (filesObj, shaMap, { statusCb, assetType }) => {
     if (Array.isArray(shaMap[fileObj.hash])) {
       shaMap[fileObj.hash].push(fileObj)
     } else {
+      // eslint-disable-next-line no-param-reassign
       shaMap[fileObj.hash] = [fileObj]
     }
     statusCb({

src/deploy/index.js

@@ -16,21 +16,24 @@ const hashFns = require('./hash_fns')
 const uploadFiles = require('./upload_files')
 const { waitForDiff, waitForDeploy, getUploadList, defaultFilter } = require('./util')
 
-const deploySite = async (api, siteId, dir, opts) => {
-  opts = {
-    fnDir: null,
-    configPath: null,
-    draft: false,
+const deploySite = async (
+  api,
+  siteId,
+  dir,
+  {
+    fnDir = null,
+    configPath = null,
+    draft = false,
     // API calls this the 'title'
-    message: undefined,
-    tmpDir: tempy.directory(),
-    deployTimeout: DEFAULT_DEPLOY_TIMEOUT,
-    concurrentHash: DEFAULT_CONCURRENT_HASH,
-    concurrentUpload: DEFAULT_CONCURRENT_UPLOAD,
-    filter: defaultFilter,
-    syncFileLimit: DEFAULT_SYNC_LIMIT,
-    maxRetry: DEFAULT_MAX_RETRY,
-    statusCb: () => {
+    message: title,
+    tmpDir = tempy.directory(),
+    deployTimeout = DEFAULT_DEPLOY_TIMEOUT,
+    concurrentHash = DEFAULT_CONCURRENT_HASH,
+    concurrentUpload = DEFAULT_CONCURRENT_UPLOAD,
+    filter = defaultFilter,
+    syncFileLimit = DEFAULT_SYNC_LIMIT,
+    maxRetry = DEFAULT_MAX_RETRY,
+    statusCb = () => {
       /* default to noop */
       // statusObj: {
       //     type: name-of-step
@@ -39,22 +42,21 @@ const deploySite = async (api, siteId, dir, opts) => {
       //     spinner: a spinner from cli-spinners package
       // }
     },
-    // allows updating an existing deploy
-    deployId: null,
-    ...opts,
-  }
-
-  const { fnDir, configPath, statusCb, message: title } = opts
-
+    deployId: deployIdOpt = null,
+    hashAlgorithm,
+    assetType,
+    branch,
+  } = {},
+) => {
   statusCb({
     type: 'hashing',
     msg: `Hashing files...`,
     phase: 'start',
   })
 
   const [{ files, filesShaMap }, { functions, fnShaMap }] = await Promise.all([
-    hashFiles(dir, configPath, opts),
-    hashFns(fnDir, opts),
+    hashFiles(dir, configPath, { concurrentHash, hashAlgorithm, assetType, statusCb, filter }),
+    hashFns(fnDir, { tmpDir, concurrentHash, hashAlgorithm, statusCb, assetType }),
   ])
 
   const filesCount = Object.keys(files).length
@@ -82,22 +84,22 @@ const deploySite = async (api, siteId, dir, opts) => {
     body: {
       files,
       functions,
-      async: Object.keys(files).length > opts.syncFileLimit,
-      branch: opts.branch,
-      draft: opts.draft,
+      async: Object.keys(files).length > syncFileLimit,
+      branch,
+      draft,
     },
   })
-  if (opts.deployId === null) {
+  if (deployIdOpt === null) {
     if (title) {
       deployParams = { ...deployParams, title }
     }
     deploy = await api.createSiteDeploy(deployParams)
   } else {
-    deployParams = { ...deployParams, deploy_id: opts.deployId }
+    deployParams = { ...deployParams, deploy_id: deployIdOpt }
     deploy = await api.updateSiteDeploy(deployParams)
   }
 
-  if (deployParams.body.async) deploy = await waitForDiff(api, deploy.id, siteId, opts.deployTimeout)
+  if (deployParams.body.async) deploy = await waitForDiff(api, deploy.id, siteId, deployTimeout)
 
   const { id: deployId, required: requiredFiles, required_functions: requiredFns } = deploy
 
@@ -111,22 +113,22 @@ const deploySite = async (api, siteId, dir, opts) => {
 
   const uploadList = getUploadList(requiredFiles, filesShaMap).concat(getUploadList(requiredFns, fnShaMap))
 
-  await uploadFiles(api, deployId, uploadList, opts)
+  await uploadFiles(api, deployId, uploadList, { concurrentUpload, statusCb, maxRetry })
 
   statusCb({
     type: 'wait-for-deploy',
     msg: 'Waiting for deploy to go live...',
     phase: 'start',
   })
-  deploy = await waitForDeploy(api, deployId, siteId, opts.deployTimeout)
+  deploy = await waitForDeploy(api, deployId, siteId, deployTimeout)
 
   statusCb({
     type: 'wait-for-deploy',
-    msg: opts.draft ? 'Draft deploy is live!' : 'Deploy is live!',
+    msg: draft ? 'Draft deploy is live!' : 'Deploy is live!',
     phase: 'stop',
   })
 
-  await rimraf(opts.tmpDir)
+  await rimraf(tmpDir)
 
   const deployManifest = {
     deployId,

src/index.js

@@ -6,36 +6,29 @@ const { addMethods } = require('./methods')
 const { getOperations } = require('./operations')
 
 class NetlifyAPI {
-  constructor(accessToken, opts) {
+  constructor(firstArg, secondArg) {
     addMethods(this)
 
     // variadic arguments
-    if (typeof accessToken === 'object') {
-      opts = accessToken
-      accessToken = null
-    }
-    // default opts
-    opts = {
-      userAgent: 'netlify/js-client',
-      scheme: dfn.schemes[0],
-      host: dfn.host,
-      pathPrefix: dfn.basePath,
-      accessToken,
-      globalParams: {},
-      ...opts,
-    }
+    const [accessTokenInput, opts = {}] = typeof firstArg === 'object' ? [null, firstArg] : [firstArg, secondArg]
 
-    this.defaultHeaders = {
-      'User-agent': opts.userAgent,
+    // default opts
+    const {
+      userAgent = 'netlify/js-client',
+      scheme = dfn.schemes[0],
+      host = dfn.host,
+      pathPrefix = dfn.basePath,
+      accessToken = accessTokenInput,
+      globalParams = {},
+      agent,
+    } = opts
+
+    const defaultHeaders = {
+      'User-agent': userAgent,
       accept: 'application/json',
     }
 
-    this.scheme = opts.scheme
-    this.host = opts.host
-    this.pathPrefix = opts.pathPrefix
-    this.globalParams = opts.globalParams
-    this.accessToken = opts.accessToken
-    this.agent = opts.agent
+    Object.assign(this, { defaultHeaders, scheme, host, pathPrefix, globalParams, accessToken, agent })
   }
 
   get accessToken() {
@@ -63,9 +56,7 @@ class NetlifyAPI {
     return `${this.scheme}://${this.host}${this.pathPrefix}`
   }
 
-  async getAccessToken(ticket, opts) {
-    opts = { poll: DEFAULT_TICKET_POLL, timeout: DEFAULT_TICKET_TIMEOUT, ...opts }
-
+  async getAccessToken(ticket, { poll = DEFAULT_TICKET_POLL, timeout = DEFAULT_TICKET_TIMEOUT } = {}) {
     const { id } = ticket
 
     // ticket capture
@@ -79,8 +70,8 @@ class NetlifyAPI {
     }
 
     await pWaitFor(checkTicket, {
-      interval: opts.poll,
-      timeout: opts.timeout,
+      interval: poll,
+      timeout,
       message: 'Timeout while waiting for ticket grant',
     })