fix: optional arguments go in the options object (#3118)

We have a few older APIs that take multiple optional arguments, which makes our code more complicated as it has to guess the users' intent, sometimes by inspecting properties on the passed args to see if they happen to correspond with properties on the actual options object.

The options object was recently added to all API calls and is the right place for optional arguments to go, so the change here is to move all optional arguments into the options object, except where the presence of an optional argument dramatically changes the behaviour of the call (`ipfs.bootstrap` I'm mostly looking at you), in which case the methods are split out into multiple versions that do distinct things.

Only the programatic API is affected, the CLI and HTTP APIs do not change.

BREAKING CHANGES:

- `ipfs.bitswap.wantlist([peer], [options])` is split into:
  - `ipfs.bitswap.wantlist([options])`
  - `ipfs.bitswap.wantlistForPeer(peer, [options])`
- `ipfs.bootstrap.add([addr], [options])` is split into:
  - `ipfs.bootstrap.add(addr, [options])` - add a bootstrap node
  - `ipfs.bootstrap.reset()` - restore the default list of bootstrap nodes
- `ipfs.bootstrap.rm([addr], [options])` is split into:
  - `ipfs.bootstrap.rm(addr, [options])` - remove a bootstrap node
  - `ipfs.bootstrap.clear([options])` - empty the bootstrap list
- `ipfs.dag.get(cid, [path], [options])` becomes `ipfs.dag.get(cid, [options])`
  - `path` is moved into the `options` object
- `ipfs.dag.tree(cid, [path], [options])` becomes `ipfs.dag.tree(cid, [options])`
  - `path` is moved into the `options` object
- `ipfs.dag.resolve(cid, [path], [options])` becomes `ipfs.dag.resolve(cid, [options])`
  - `path` is moved into the `options` object
- `ipfs.files.flush([path], [options])` becomes `ipfs.files.flush(path, [options])`
- `ipfs.files.ls([path], [options])` becomes `ipfs.files.ls(path, [options])`
- `ipfs.object.new([template], [options])` becomes `ipfs.object.new([options])`
  - `template` is moved into the `options` object
- `ipfs.pin.ls([paths], [options])` becomes `ipfs.pin.ls([options])`
  - `paths` is moved into the `options` object

Co-authored-by: Hugo Dias <[email protected]>

Author: achingbrain

README.md

@@ -280,7 +280,7 @@ const ipfs = ipfsClient({ host: '1.1.1.1', port: '80', apiPath: '/ipfs/api/v0' }
 ```javascript
 const bitswap = require('ipfs-http-client/src/bitswap')('/ip4/127.0.0.1/tcp/5001')
 
-const list = await bitswap.wantlist(key)
+const list = await bitswap.wantlist()
 // ...
 ```
 

src/bitswap/index.js

@@ -2,6 +2,7 @@
 
 module.exports = config => ({
   wantlist: require('./wantlist')(config),
+  wantlistForPeer: require('./wantlist-for-peer')(config),
   stat: require('./stat')(config),
   unwant: require('./unwant')(config)
 })

src/bitswap/wantlist-for-peer.js

@@ -0,0 +1,23 @@
+'use strict'
+
+const CID = require('cids')
+const configure = require('../lib/configure')
+const toUrlSearchParams = require('../lib/to-url-search-params')
+
+module.exports = configure(api => {
+  return async (peerId, options = {}) => {
+    peerId = typeof peerId === 'string' ? peerId : new CID(peerId).toString()
+
+    const res = await (await api.post('bitswap/wantlist', {
+      timeout: options.timeout,
+      signal: options.signal,
+      searchParams: toUrlSearchParams({
+        ...options,
+        peer: peerId
+      }),
+      headers: options.headers
+    })).json()
+
+    return (res.Keys || []).map(k => new CID(k['/']))
+  }
+})

src/bitswap/wantlist.js

@@ -5,16 +5,7 @@ const configure = require('../lib/configure')
 const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
-  return async (peer, options = {}) => {
-    if (peer && (peer.timeout || peer.signal)) {
-      options = peer
-      peer = undefined
-    }
-
-    if (peer) {
-      options.peer = typeof peer === 'string' ? peer : new CID(peer).toString()
-    }
-
+  return async (options = {}) => {
     const res = await (await api.post('bitswap/wantlist', {
       timeout: options.timeout,
       signal: options.signal,

src/bootstrap/clear.js

@@ -0,0 +1,20 @@
+'use strict'
+
+const configure = require('../lib/configure')
+const toUrlSearchParams = require('../lib/to-url-search-params')
+
+module.exports = configure(api => {
+  return async (options = {}) => {
+    const res = await api.post('bootstrap/rm', {
+      timeout: options.timeout,
+      signal: options.signal,
+      searchParams: toUrlSearchParams({
+        ...options,
+        all: true
+      }),
+      headers: options.headers
+    })
+
+    return res.json()
+  }
+})

src/bootstrap/index.js

@@ -2,6 +2,8 @@
 
 module.exports = config => ({
   add: require('./add')(config),
+  clear: require('./clear')(config),
   rm: require('./rm')(config),
+  reset: require('./reset')(config),
   list: require('./list')(config)
 })

src/bootstrap/reset.js

@@ -0,0 +1,20 @@
+'use strict'
+
+const configure = require('../lib/configure')
+const toUrlSearchParams = require('../lib/to-url-search-params')
+
+module.exports = configure(api => {
+  return async (options = {}) => {
+    const res = await api.post('bootstrap/add', {
+      timeout: options.timeout,
+      signal: options.signal,
+      searchParams: toUrlSearchParams({
+        ...options,
+        default: true
+      }),
+      headers: options.headers
+    })
+
+    return res.json()
+  }
+})

src/dag/get.js

@@ -15,13 +15,8 @@ module.exports = configure((api, options) => {
   const getBlock = require('../block/get')(options)
   const dagResolve = require('./resolve')(options)
 
-  return async (cid, path, options = {}) => {
-    if (path && typeof path === 'object') {
-      options = path
-      path = null
-    }
-
-    const resolved = await dagResolve(cid, path, options)
+  return async (cid, options = {}) => {
+    const resolved = await dagResolve(cid, options)
     const block = await getBlock(resolved.cid, options)
     const dagResolver = resolvers[block.cid.codec]
 

src/dag/resolve.js

@@ -5,17 +5,12 @@ const configure = require('../lib/configure')
 const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
-  return async (cid, path, options = {}) => {
-    if (path && typeof path === 'object') {
-      options = path
-      path = null
-    }
-
+  return async (cid, options = {}) => {
     const res = await api.post('dag/resolve', {
       timeout: options.timeout,
       signal: options.signal,
       searchParams: toUrlSearchParams({
-        arg: path ? [cid, path].join(path.startsWith('/') ? '' : '/') : `${cid}`,
+        arg: `${cid}${options.path ? `/${options.path}`.replace(/\/[/]+/g, '/') : ''}`,
         ...options
       }),
       headers: options.headers

src/files/flush.js

@@ -6,9 +6,8 @@ const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
   return async (path, options = {}) => {
-    if (typeof path !== 'string') {
-      options = path || {}
-      path = '/'
+    if (!path || typeof path !== 'string') {
+      throw new Error('ipfs.files.flush requires a path')
     }
 
     const res = await api.post('files/flush', {

src/files/ls.js

@@ -7,9 +7,8 @@ const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
   return async function * ls (path, options = {}) {
-    if (typeof path !== 'string') {
-      options = path || {}
-      path = '/'
+    if (!path || typeof path !== 'string') {
+      throw new Error('ipfs.files.ls requires a path')
     }
 
     const res = await api.post('files/ls', {

src/object/new.js

@@ -5,17 +5,12 @@ const configure = require('../lib/configure')
 const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
-  return async (template, options = {}) => {
-    if (typeof template !== 'string') {
-      options = template || {}
-      template = null
-    }
-
+  return async (options = {}) => {
     const res = await api.post('object/new', {
       timeout: options.timeout,
       signal: options.signal,
       searchParams: toUrlSearchParams({
-        arg: template,
+        arg: options.template,
         ...options
       }),
       headers: options.headers

src/pin/ls.js

@@ -5,20 +5,17 @@ const configure = require('../lib/configure')
 const toUrlSearchParams = require('../lib/to-url-search-params')
 
 module.exports = configure(api => {
-  return async function * ls (path, options = {}) {
-    if (path && (path.type || path.timeout)) {
-      options = path || {}
-      path = []
+  return async function * ls (options = {}) {
+    if (options.paths) {
+      options.paths = Array.isArray(options.paths) ? options.paths : [options.paths]
     }
 
-    path = Array.isArray(path) ? path : [path]
-
     const res = await api.post('pin/ls', {
       timeout: options.timeout,
       signal: options.signal,
       searchParams: toUrlSearchParams({
-        arg: path.map(p => `${p}`),
         ...options,
+        arg: (options.paths || []).map(path => `${path}`),
         stream: true
       }),
       headers: options.headers