Repos, repos, repos

[achingbrain]

Making changes in js-ipfs typically involves changing this repo, the ipfs-http-client, interface-js-ipfs-core and also ipfds-ctl (because of it's dependency on ipfs-http-client.

We have these as separate modules but they are really modular in name only due to the cross-dependencies indicated above.

This has had a number of consequences:

1. Developers have to coordinate PRs across all of these repos which is painful for experienced contributors and completely bewildering for new contributors
2. It's impossible to have one PR that encapsulates a changeset - this makes it hard to test, hard to review, hard to integrate, hard to release and hard to roll back
3. There is no one build that says "Yes, this change is ok" which results in a lot of manual work and makes automation harder because you have to juggle multiple versions of multiple modules
4. There is a large amount of duplication across these repos, from tests to utility methods (js-ipfs-utils is helping with the utility methods but is also compounding the problem).
5. Coordinating shared dependency updates across these modules becomes a Byzantine task (e.g. see how out of date libp2p, ipld, are at the moment etc)

Since this project does not include the libp2p and IPLD codebases it should be a relatively thin layer on top of them, yet development is very complex.

We've started talking about re-integrating some core modules in #2222 but we should go further than that.

⚒️ Break the ipfsd-ctl/ipfs-http-client dependency

Possible solutions:

a. Make ipfs-http-client a peer dependency of ipfsd-ctl
b. Require an implementation to be passed in the same way as with in-proc js-ipfs daemons

💍 One PR to rule them all

Combine the repos. One way to do this might be adding the client/ipfsd to core:

const IPFS = require('ipfs')
const IPFSClient = require('ipfs/client')

...

✅ Pro: Simple to use
✅ Pro: Works with existing unspecified tooling that assumes one-module-per-repo
❌ Con: If you only consume the client you'll end up installing lots of dependencies you don't need
❌ Con: Separate PRs to interface-js-ipfs-core still required unless you add that in too
❌ Con: Poor separation of concerns
❌ Con: All-or-nothing releases

Another way might be a lerna-style mono-repo a la babel:

$ ls -l
js-ipfs/
├── package.json
├── lerna.json
└── packages/
    ├── ipfs
    ├── ipfsd-ctl
    ├── ipfs-http-client
    └── interface-js-ipfs-core

✅ Pro: One change, one PR
✅ Pro: Existing dependant code does not break because module names do not change
✅ Pro: Still able to release individual modules independently
✅ Pro: Easier to make, review and release changes
✅ Pro: Easier to contribute to
✅ Pro: The tooling figures out cross-package dependencies, no more npm link required
✅ Pro: Clever enough to figure out which package depends on which and only test/release what's changed
❌ Con: In the worst case tests take as long as the slowest module
❌ Con: Cannot group GH issues by package
❌ Con: Tools that assume one-module-per-repo will need fixing
❌ Con: When adding new dependencies, it's easy to forget to add them to the right package due to dependency hoisting

cc @ipfs/wg-js-core and @ipfs/repos-javascript

[hugomrdias]

Con: When adding new dependencies, it's easy to forget to add them to the right package due to dependency hoisting

Aegir to the rescue aegir deps running in CI

[vasco-santos]

❌ Con: Cannot group GH issues by package

I also see this as an opportunity to start using labels instead. IMHO, one single repo with issues, which we can filter through tags is more efficient to maintain

[achingbrain]

one single repo with issues, which we can filter through tags is more efficient to maintain

I can see that both ways, and it's been a sticking point for adoption in the past, but I do notice that js-ipfs is the gateway for most issues/bug reports for IPFS/IPLD/libp2p so part of maintaining it would be to triage incoming issues and move them to the appropriate repos (e.g. libp2p/js-libp2p, ipld/js-ipld, etc) so it's not like it'd be just one repo with hundreds of issues.

[daviddias]

On:

You can pass a http-api-client today as an argument:

IpfsClient - A custom IPFS API constructor to use instead of the packaged one

https://github.com/ipfs/js-ipfsd-ctl/#ipfsfactory---const-f--ipfsfactorycreateoptions

So it should just be a question of upgrading the tests to do it

[achingbrain]

N.b 'you can pass' is not the same as 'require an implementation to be passed'. Sometimes you've got to be cruel to be kind.

[daviddias]

How does it differ for the goal you described?

[achingbrain]

We'd remove js-ipfs-http-client as a dependency and throw an error if an implementation wasn't passed. The developer would be faced with a failing test that's at least easy to fix and we wouldn't see as much churn in js-ipfsd-ctl releases.

[daviddias]

@achingbrain what about the 260 repos using it -- https://github.com/ipfs/js-ipfsd-ctl/network/dependents?package_id=UGFja2FnZS0xNDQ1MDU1Mw%3D%3D -- plus the ones in the future? Is it worth adding one more step for them in trade for us having a angry test script shouting so that we don't forget a good practice? I'm sure we can do this in a better way.

[achingbrain]

Most of those repos seem to use ipfsd-ctl as a convenience to start an IPFS node in one step, which I guess is more of a reflection on our API than anything else.

This is (as I see it) the least contentious part of this conversation, shall we take it to a dedicated issue?

[achingbrain]

An example of a contributor who is confused by our module setup: ipfs-inactive/js-ipfs-http-client#1034

If this were one repo with one PR-per-change, he would find it easier to contribute.

[achingbrain]

You can pass a http-api-client today as an argument:

Just noticed this isn't even the case for all IPFSd types (at the time of writing) - Client and Daemon types support IpfsClient, for InProc the opt is called IpfsApi. 🤦‍♂

[achingbrain]

An example of the problem: right now there are changes merged into interface-js-ipfs-core master to support a PR to js-ipfs - they require the PR to be merged for the tests to run successfully.

Other PRs to js-ipfs that need changes made to interface-js-ipfs-core are now blocked by the first PR.

One solution is to have the deps in your js-ipfs PR point to branches but this just delays the problem, as someone still has to sit down and merge & release all the dep branches - if problems occur during this process the other PRs become blocked again.