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

Discussion: IPFS.create({ ... }) should return same API regardless of the init and start options. #3285

Closed
Gozala opened this issue Sep 14, 2020 · 3 comments
Assignees
Labels
kind/bug A bug in existing code (including security flaws) kind/resolved-in-helia status/in-progress In progress

Comments

@Gozala
Copy link
Contributor

Gozala commented Sep 14, 2020

At the moment IPFS.create will return API object that has different interface depending on init and start options:

const { api } = apiManager.update({
init: Components.init({ apiManager, print, options }),
dns: Components.dns(),
isOnline: Components.isOnline({ libp2p: undefined })
}, async () => { throw new NotInitializedError() }) // eslint-disable-line require-await
const initializedApi = options.init && await api.init()
const startedApi = options.start && initializedApi && await initializedApi.start()
/**
* @template T, THEN, ELSE
* @typedef {NonNullable<T> extends false
* ? THEN : ELSE } IsFalse
*/
/** @type {IsFalse<INIT, typeof api, IsFalse<START, typeof initializedApi, typeof startedApi>>} */
// @ts-ignore
const ipfs = startedApi || initializedApi || api
return ipfs

It is kind of cool that one could do that in JS (and even make TS infer it properly), but I think it has enough drawbacks that I would like to make an argument against it:

  1. It is difficult to document this kind of behavior. In fact documentation seems to incorrectly suggest that it returned object will implement IPFS Core API. And only vaguely mentions that initialization is different from creating because many special properties are set during initialization.
  2. Unsurprisingly (per @lidel) some (of even our) code assumes that returned API object implements same interface.
  3. This introduces side effects into the system. Some APIs will not be available before init and will be after init. This specific aspet is impossible to type in TS because types can just change in side effect of a call.
  4. I think this is primary reason for ApiManager which introduces quite a bit complexity in exchange for unclear (at least to me) benefits.

I understand that IPFS node can not provide all the core functionality when it's not initialized / started. However I would propose to either (or both):

  1. Have the same core API and throw error e.g. not initialized / not started error(s) when specific functionality is in-operational.
  2. Have a way to distinguish between 'unitialized | started | initialized' states so that function that is passed IPFS API object can asses what API subset can be used (without try catch or subapi in)
  3. Have a different factory functions, for different API objects.
  4. Remove side effects e.g. api.init() can return InitilaizedAPI instance, without mutating the api itself.
@Gozala Gozala added the need/triage Needs initial labeling and prioritization label Sep 14, 2020
@achingbrain achingbrain added kind/bug A bug in existing code (including security flaws) and removed need/triage Needs initial labeling and prioritization labels Sep 17, 2020
@Gozala Gozala added the status/in-progress In progress label Sep 17, 2020
@achingbrain
Copy link
Member

This is a bug. The API should be the same regardless of the node state, and any methods that require initialisation or a running node should throw if invoked.

I did some work to address this in #2762 which will be simpler to land after #3288.

@Gozala Gozala self-assigned this Oct 9, 2020
@Gozala
Copy link
Contributor Author

Gozala commented Oct 9, 2020

I'll work on this after typescript patches land

@SgtPooki
Copy link
Member

js-ipfs is being deprecated in favor of Helia. You can #4336 and read the migration guide.

Please feel to reopen with any comments by 2023-06-02. We will do a final pass on reopened issues afterward (see #4336).

This issue is most likely resolved in Helia, please try it out!

@github-project-automation github-project-automation bot moved this from 🥞 Todo to ✅ Done in js-ipfs deprecation May 26, 2023
@SgtPooki SgtPooki assigned SgtPooki and unassigned achingbrain May 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws) kind/resolved-in-helia status/in-progress In progress
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants