Provide access to bundled libraries when run in browser

[lidel]

I wonder if there are any cons to exposing libraries such as is-ipfs, multiaddr and multihashes under ipfs.util.lib.* or maybe only in browser-specific build under windows.ipfsLibs ?

I feel people who use browserified artifact would really appreciate it as it would remove code duplication in smaller projects (no need to ship library twice just to have it available as window.foo).

Perhaps there are better ways to do this that I am not aware of? Any thoughts on this?

[daviddias]

Related with ipfs/js-ipfs#525 (comment)

@lidel wanna help us achieve both?

[lidel]

I am sorry, I have limited free time and need to prioritize my activities.
This is not a blocker for me, just an inconvenience, so it does not have high priority :-)
WebExtension migration is my main focus right now.

[vasco-santos]

Hello @diasdavid

I read the Issue 525, particularly your comment previously referenced and the PRs associated. I have been analyzing how to implement this feature for both js-ipfs and js-ipfs-api.

1. In the js-ipfs, I am thinking on the following:

https://github.com/ipfs/js-ipfs/blob/master/src/core/index.js#L131
After this line, add an export to the util object:

exports.util = {
  Buffer: Buffer,
  crypto: crypto,
  dagPB: dagPB,
  dagCBOR: dagCBOR,
  multibase: multibase,
  multihash: multihash,
  stream: stream
}

Usage:

const ipfs = require('ipfs')
const util = ipfs.util

even cleaner using imports:

import IPFS, { util } from 'ipfs';

In spite of being possible to provide the util object inside the class using this.util, I consider this to be the easiest way to provide the util object to js-ipfs consumer.

2. For the js-ipfs-api repo, I wanted to follow the same strategy for consistency. However, this repo already has Utility Functions provided.

Taking into account the second point, I think for consistency among repos, I should expose the util inside the class for the js-ipfs and add it to Utility Functions for the js-ipfs-api.

What do you think?

[daviddias]

We should call it types instead of util. Another option being or util.types

[vasco-santos]

So, in the js-ipfs already exists the types in the class instance. Can I add the remaining ones in there?

For the js-ipfs-api repo, I will create the types as in the other repo.

[daviddias]

Go for it :)

[alanshaw]

For ipfs-postmsg-proxy I'm having to add ipld-dag-cbor, multibase, multihashes, libp2p-crypto and is-ipfs as direct dependencies although the library does not use them and has no guarantee that any consumers of the library use them.

It also doesn't depend on js-ipfs so these dependencies definitely would not normally be in the bundle unless the consumer app uses them. I feel like this is bloating the bundle size unnecessarily.

The exact same thing happened in the PR to js-ipfs-api.

In the case of js-ipfs, it is worth it for convenience since they are already bundled, but adding unused dependencies doesn't feel right and is not in the spirit of the issue title "Provide access to bundled libraries when run in browser".

[vasco-santos]

Hey @alanshaw

I agree that there are several cases where the bundle size was increased with no benefit. That's why I mentioned the increase in the bundle size on the PR. However, I think that if we support this on js-ipfs, we have to support this on js-ipfs-api for consistency. Therefore, we should support it in both, or in none of them.

Moreover, I also suggested removing the Buffer in a breaking change, since it is automatically added by the bundlers nowadays. This way, we could also decrease the bundle size removing it.

[daviddias]

We need that tree-shaking, it will come once ESM support is everywhere :)

[mitra42]

I just found this, I'm curious why it was defined based on an instance of ipfs, rather than something static since it points at the statically defined types.

My specific use case is to do something like foo instanceof ipfs.types.CID but it happens in static functions or places where the ipfs instance isn't passed as a variable, i.e. where I don't easily have access to the "ipfs" variable, but have easy access to the result of
IPFS = require('ipfs'),

The current code does CID=require('cids') solely to allow this type check, which seems exactly the kind of example this change was intended to avoid.