Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: optional arguments go in the options object #3118

Merged
merged 2 commits into from
Jul 2, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Jun 24, 2020

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

@achingbrain achingbrain requested a review from hugomrdias June 24, 2020 11:25
@achingbrain
Copy link
Member Author

Follows on from #3071

@achingbrain achingbrain marked this pull request as ready for review June 30, 2020 12:52
@achingbrain achingbrain force-pushed the feat/optional-args-go-in-the-options-object branch 2 times, most recently from 805c2fd to 52e04f6 Compare July 1, 2020 11:44
docs/core-api/BITSWAP.md Outdated Show resolved Hide resolved
docs/core-api/BOOTSTRAP.md Outdated Show resolved Hide resolved
docs/core-api/BOOTSTRAP.md Show resolved Hide resolved
docs/core-api/BOOTSTRAP.md Outdated Show resolved Hide resolved
docs/core-api/DAG.md Show resolved Hide resolved
packages/interface-ipfs-core/src/bitswap/utils.js Outdated Show resolved Hide resolved
packages/ipfs-http-client/src/files/ls.js Outdated Show resolved Hide resolved
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 paroperties 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 <hugomrdias@gmail.com>
@achingbrain achingbrain force-pushed the feat/optional-args-go-in-the-options-object branch from 51d2b0b to 8e4af64 Compare July 2, 2020 08:31
@achingbrain achingbrain merged commit 8cb8c73 into master Jul 2, 2020
@achingbrain achingbrain deleted the feat/optional-args-go-in-the-options-object branch July 2, 2020 13:47
@github-actions github-actions bot mentioned this pull request Feb 4, 2022
SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
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 <hugomrdias@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants