Skip to content

Commit

Permalink
fix: optional arguments go in the options object (#3118)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
achingbrain and hugomrdias authored Jul 2, 2020
1 parent a2db0fa commit 86ea629
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 45 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()
// ...
```

Expand Down
1 change: 1 addition & 0 deletions src/bitswap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
23 changes: 23 additions & 0 deletions src/bitswap/wantlist-for-peer.js
Original file line number Diff line number Diff line change
@@ -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['/']))
}
})
11 changes: 1 addition & 10 deletions src/bitswap/wantlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions src/bootstrap/clear.js
Original file line number Diff line number Diff line change
@@ -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()
}
})
2 changes: 2 additions & 0 deletions src/bootstrap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
20 changes: 20 additions & 0 deletions src/bootstrap/reset.js
Original file line number Diff line number Diff line change
@@ -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()
}
})
9 changes: 2 additions & 7 deletions src/dag/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
9 changes: 2 additions & 7 deletions src/dag/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions src/files/flush.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
5 changes: 2 additions & 3 deletions src/files/ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
9 changes: 2 additions & 7 deletions src/object/new.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 4 additions & 7 deletions src/pin/ls.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 86ea629

Please sign in to comment.