Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we bundle js-ipfs? #6

Closed
hacdias opened this issue Oct 23, 2019 · 6 comments · Fixed by #17
Closed

Should we bundle js-ipfs? #6

hacdias opened this issue Oct 23, 2019 · 6 comments · Fixed by #17

Comments

@hacdias
Copy link
Member

hacdias commented Oct 23, 2019

I've noticed that, right now, ipfs-cohost is bundling js-ipfs with it. If we look at it as a library, it does not make sense to have it as a dependency because it will only add up in size.

Also, as a standalone executable, I wonder if we need js-ipfs. I believe most people using this tool will already have an IPFS node running and it might not make much sense to be spinning a new one just to cohost a website, which, by default, will be a lazy cohost (see #5).

/cc @lidel @autonome

@autonome
Copy link

What is the concern over bundling a node? Only size? Or other things like cpu and bandwidth too?

At the very least, if the bundled node is removed, the tool should clearly spell out the recommended steps to installing a node when one is not discovered.

Aside from this particular decision, in general I recommend always assuming the user does not have a node running as the starting point in our architecture and design.

@hacdias
Copy link
Member Author

hacdias commented Oct 24, 2019

My main concern was the bundle size and time it would take to install js-ipfs and all of its dependencies. I'm thinking either about the case where the user wants this as a library or when they use this as a CLI via npx ipfs-cohost. It could take a bit to install js-ipfs just to connect to their local daemon.

At the very least, if the bundled node is removed, the tool should clearly spell out the recommended steps to installing a node when one is not discovered.

Yes, why not? Also, I don't see why many people would want to cohost websites on an offline node.

Aside from this particular decision, in general I recommend always assuming the user does not have a node running as the starting point in our architecture and design.

I agree. Why not using js-ipfsd-ctl to connect to an already existing daemon or running one if installed? I just don't think it's worth it to bundle an entire implementation when this is a tool that's supposed to connect to the user's already existing repository.

@lidel
Copy link
Member

lidel commented Oct 24, 2019

The problem with bundling js-ipfs with ipfs-cohost is that we kinda encourage running multiple nodes as the default instead of incentivizing users to install js-ipfs/go-ipfs/ipfs-desktop.

I believe we should default to trying HTTP API first:

  • When run via CLI try to reuse existing node over HTTP API
    • support --api parameter in case someone wants to provide multiaddr by hand
    • if no --api, check $IPFS_HOME/api, then try default port, then Addresses.API in $IPFS_HOME/config
  • When no HTTP API is available return an error informing user that no daemon was found and suggest installing js-ipfs/go-ipfs or ipfs-desktop (with links), or trying again with explicit --api
    • If we really want, we could add --embedded to the list of those options.
      It would spawn embedded js-ipfs, enabling one-stop-shop experience. But it should not be an opt-in.

As for the bundle size, we need to pick one of below options:

(A) keep cli and lib together

  • Remove js-ipfs as a dependency, replace it with js-ipfs-http-client
  • Ensure the library does not care if ipfs API object is js-ipfs or js-ipfs-http-client
    (this is already implemented in PR feat: match spec #5)

Potential problem is that CLI needs ipfs-http-client as a dependency, so we trade js-ipfs for js-ipfs-http-client and still increase the bundle size of the library itself.

(B) move cli or lib to a separate package

We could go with ipfs-cohost-cli or ipfs-cohost-api


Thoughts?

@hacdias
Copy link
Member Author

hacdias commented Oct 24, 2019

I wouldn't mind to separate them, but I would be careful about the namings. Perhaps we could even keep them both on the same repository for the sake of simplicity.

I don't want to have people run npx ipfs-cohost-cli. That doesn't look good and I would feel that a bit unintuitive. Perhaps the CLI would keep being ipfs-cohost and the lib/api ipfs-cohost-api or ipfs-cohost-lib.

@hacdias hacdias mentioned this issue Oct 24, 2019
@hacdias
Copy link
Member Author

hacdias commented Oct 30, 2019

Something else worth nothing: npx ipfs-cohost takes quite a bit and I believe one of the reason's might be js-ipfs as it has dependencies that are required to be built

@olizilla
Copy link
Member

Yep, it use ipfs-provider which will try the http api first and then fallback to js-ipfs otherwise https://github.com/ipfs-shipyard/ipfs-provider

It would be great if we could lazily fetch js-ipfs if needed. Running from npx is crushingly slow due to the js-ipfs dep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants