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

webui and ipfs-desktop do depend on js-ipfs/js-ipfs-http-api #2542

Closed
daviddias opened this issue Oct 16, 2019 · 4 comments
Closed

webui and ipfs-desktop do depend on js-ipfs/js-ipfs-http-api #2542

daviddias opened this issue Oct 16, 2019 · 4 comments

Comments

@daviddias
Copy link
Member

daviddias commented Oct 16, 2019

From https://github.com/ipfs/js-ipfs/blob/master/docs/RELEASE_ISSUE_TEMPLATE.md

image

I don't believe this is correct. Because:

  • webui depends on ipfs-redux-bundle, which depends on js-ipfs-http-client
  • ipfs-desktop depends on js-ipfsd-ctl that uses js-ipfs-http-client

also:

  • js-ipfs does expose webui to its users through its own http gateway.
@achingbrain
Copy link
Member

These modules need to declare which versions of IPFS and/or the HTTP API client they are compatible with, even if it's just for their tests, otherwise they risk depending on features that may not be available at runtime and suffering other hard to replicate bugs. cc @hacdias @olizilla

webui

We could add ipfs-redux-bundle to our external repo testing since it has a direct dependency we can swap out?

Otherwise the webui end to end tests could be configured to run against an actual IPFS instance, the version of which is declared in package.json which would also let us run them against js-IPFS PRs/releases as part of CI.

ipfs-desktop

This module should pass a known version of ipfs-http-client to ipfsd-ctl, then we can add it to our external repo testing.

If it just takes whatever ipfsd-ctl happens to be bundled with, then it's the wild west.

webui on the gateway

js-ipfs does expose webui to its users through its own http gateway.

This is true, but we resolve the CID of a released version*, so assume that some testing has taken place prior to the PR to js-IPFS updating the CID was submitted, in the same way that we accept other dependency updates.

This is a gap though, maybe we could figure out a way of running the webui tests with the current branch head as part of a release. Might involve doing something clever with connect-deps and ipfs-redux-bundle or the e2e test refactor suggested above.


@achingbrain
Copy link
Member

@hacdias @olizilla any thoughts on this? Specifically about changing webui/desktop to test against an actual version of js-ipfs that we can swap out during CI.

achingbrain added a commit to ipfs/aegir that referenced this issue Oct 21, 2019
Goes some way to address ipfs/js-ipfs#2542 in that it changes our
definition of how a module 'depends' on IPFS.  If it's in your
`node_modules` folder, it'll get upgrade to the current release we
are testing.

This means if we're testing module `A` and `A` has a depdendency
graph like `A -> B -> C -> ipfs`, as long as `ipfs` has been hoisted
to the the top level `node_modules` folder, we'll swap it out with
the new version we're trying to release.

It also dedupes `ipfs` and `ipfs-http-client` in the dep tree so
we can be sure we're testing against our `rc`, though this may
cause problems where intermediate deps need updating, but at least
the need for them to be updated will  be visible.
@hugomrdias
Copy link
Member

We should only tests direct dependents!!
For webui we should only tests ipfs-redux-bundle
For desktop it should declare ipfs and http-client and pass those on to ipfsd-ctl (this should be best practice to use ctl the new version I'm working on will have this first thing in the readme) and then we can test it directly.

@SgtPooki
Copy link
Member

SgtPooki commented Nov 3, 2022

FYI that this issue is no longer relevant since we're pulling js-ipfs out of webui and desktop (ipfs/kubo#9125). closing this.

cc @tinytb @achingbrain

@SgtPooki SgtPooki closed this as completed Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants