fix: remove non default ipld formats in the browser

[hugomrdias]

This PR removes non default ipld formats in the browser, these formats are extremely big, some of them aren't finished and are not commonly used in the wild.
Considering that we now have options to add support for them in the config passed to the IPFS constructor in both sync and async ways, we should let the developer choose which formats to support and make their bundle smaller and faster to load.

Documentation was added to explain how to handle the ipld config in several ways.

[vmx]

Thanks!

[lidel]

LGTM, but lets cc @olizilla as this impacts IPLD Explorer in Web UI

[olizilla]

This is the wonderful documenting @hugomrdias, I love it!

[olizilla]

This is rad. 👍 I look forward to tweaking IPLD explorer for this change.

The only thing that could make it better is a mechanim that would allow the user to provided custom implementations of dag-pb and dag-cbor and dag-raw. I don't know if that's a feature that anyone wants, but as js-ipld will throw if it finds duplicate handlers for a single format, and the defaults are added regardless, it's the only issue that occured to me as I read the (beautiful) code.

[vasco-santos]

🚀

[hugomrdias]

This is rad. 👍 I look forward to tweaking IPLD explorer for this change.

The only thing that could make it better is a mechanim that would allow the user to provided custom implementations of dag-pb and dag-cbor and dag-raw. I don't know if that's a feature that anyone wants, but as js-ipld will throw if it finds duplicate handlers for a single format, and the defaults are added regardless, it's the only issue that occured to me as I read the (beautiful) code.

They actually are not added regardless in IPLD, but i forced it on IPFS side with this PR. merge-options doesn't concat arrays by default so if the user does this:

new IPLD({
	formats: [require('ipld-git'])
})

He only gets git and NOT pb, cbor and raw but now with IPFS the user gets pb, cbor, raw and git.
We can remove this behaviour and keep it like IPLD, that way the user can provide custom cbor implementation but he will be forced to add all formats to the array. It's a tradeoff but i think this should be discussed in IPLD solved there then we can do whatever changes here.

Regarding your comment on custom dag-pb it's a design problem format and addFormat work the same way internally but they don't teach the user that we map string identifiers or multicodec constants to implementations like git-raw -> ipld-git.
One solution would be to not rely on the options passed to the constructor to add pb, cbor and raw but dynamically default to them here if everything else fails to load the code needed. /cc @vmx

[alanshaw]

I'm 👍 on making the bundle size smaller and I'm 👍 on providing a build of IPFS that doesn't have these IPLD formats for developers who do not use and will never use other IPLD formats.

I'm 👎 on removing them by default because it won't help IPLD adoption. It raises a barrier for people experimenting with IPLD that the project just doesn't need at this point in it's life. People should just be able to fetch a git commit or an ethereum block from their web app if they want to and experience the magic without having to learn about IPLD format modules and how to programmatically load them.

The documentation in this PR is great, but do you think that someone putting a script tag on their page and trying to dag.get a CID for a git commit is going to come back to the js-ipfs README, find that section, read it all and then update their code? I think that's a big ask.

In general the default IPFS you get when you require or download from a CDN should be the full fat version, and as developers get more knowledgeable in the IPFS ecosystem they should be able to tailor the build for their needs.

IMHO the solution to the problem of the IPLD modules being big or incomplete is not to remove them from IPFS (or even IPLD for that matter) but to fix them. I feel like we're focusing our energies in the wrong place.

That said, what if we iterate on this PR and add an additional browser build that doesn't include the more exotic IPLD formats and a require('ipfs/lite') so people bundling their own can also get the benefits? Would that be a reasonable compromise?

There seems to be a lot of support for this PR so I'm not going to stand in the way of merging this if the @ipfs/javascript-team team really believes it's necessary. If you're in agreement with the compromise I suggested above (or something similar) then please vote with a 👍 and if you disagree and would prefer to merge this as is then 👎 on this comment and I'll get it done.

[hugomrdias]

I'm 👎 on removing them by default because it won't help IPLD adoption. It raises a barrier for people experimenting with IPLD that the project just doesn't need at this point in it's life. People should just be able to fetch a git commit or an ethereum block from their web app if they want to and experience the magic without having to learn about IPLD format modules and how to programmatically load them.

The documentation in this PR is great, but do you think that someone putting a script tag on their page and trying to dag.get a CID for a git commit is going to come back to the js-ipfs README, find that section, read it all and then update their code? I think that's a big ask.

This is basically the standard problem of magic vs control, IMO we should go for control because were are coding the build blocks of the decentralized web, magic should come in with the devs building apps and frameworks with our code. These devs will want control over magic.
Still i agreed we should also think about other user types and for them we should look at how other projects do it.
One solution i really like is how react and jest do it, it's both simple and elegant, we should just write awesome errors messages that explain the problem and present the solution with links to the documentation.

In general the default IPFS you get when you require or download from a CDN should be the full fat version, and as developers get more knowledgeable in the IPFS ecosystem they should be able to tailor the build for their needs.

We can make our bundle the one used when loaded using UNPKG with a script tag, include all the formats easily using .aegir.js to add an alias for webpack.

IMHO the solution to the problem of the IPLD modules being big or incomplete is not to remove them from IPFS (or even IPLD for that matter) but to fix them. I feel like we're focusing our energies in the wrong place.

Agreed, still i don't think we should force all users to include them, goes back to control over magic.

That said, what if we iterate on this PR and add an additional browser build that doesn't include the more exotic IPLD formats and a require('ipfs/lite') so people bundling their own can also get the benefits? Would that be a reasonable compromise?

For me this would raise more questions in the user mind than a well written error message with a link to the simple solution.
IMO we should only have one bundle for the CDN and one entry point in the code, more than that would create fragmentation in the issues, complexity in the code, etc.

[olizilla]

Good point! I'm very much favour of fixing the IPLD formats. Alan's point about focusing our efforts on where they can do most good is important. IPLD Explorer and anyone wanting to experience the magic wont get much futher than files, cbor and git today, regardless of if they are bundled by default, or have clear instructions on how to add them.

But perf and bundle size is important too, and the browser runtime is arguably more important to us than node is. I'm in favour of not supporting all formats by default in js-ipfs.

I'd be in favour of a compromise like including raw, cbor, pb andipld-git by default so we could show the cross-system magic without config, whilst dropping eth-*, bitcoin and zcash, along with adding great docs and clear errors with instructions on how to add them.

My real dream...

...is that we pull out the `dag` api entirely to it's own tool called `ipld` and we focus the ipfs story on files, and then follow up with "you want arbitrary, cross-system, merkle graphs with friends!? we got you! try out the `ipld` tool.

[mikeal]

FYI, we’re working on a new JS stack for IPLD authoring https://github.com/ipld/js-ipld-stack

One change is that the Block interface dynamically loads the necessary codec using a new library called get-codec. My intention is to keep this module very small and have different entry points for Node.js and browsers, with the browser version dynamically loading every codec (except raw cause it’s only like 4 lines in the new interface). This will remove the need for consumers to do dynamic loading of these codecs.

Update: here’s a more complete sample of what I’m thinking for get-codec https://github.com/ipld/js-ipld-stack/tree/master/src/get-codec

[alanshaw]

We can make our bundle the one used when loaded using UNPKG with a script tag, include all the formats easily using .aegir.js to add an alias for webpack.

I think that's a good compromise - can we do this please @hugomrdias?

To confirm:

• In Node.js require('ipfs')
  • all IPLD formats included
• In browser application bundle require('ipfs') bundled with webpack/browserify/etc.
  • only ipld-dag-pb, ipld-dag-cbor and ipld-raw included
• CDN bundle <script src="https://unpkg.com/ipfs/dist/index.min.js"></script>
  • all IPLD formats included

[alanshaw]

FYI, we’re working on a new JS stack for IPLD authoring https://github.com/ipld/js-ipld-stack

@mikeal that's rad! I'm excited for get-codec.

Did you see #1830? Similar kind of idea but loading directly from IPFS. It had significant push back (see the PR comments) so my next step was to pull it out into a module people could use, similar to get-codec but I didn't get round to it yet 😢.

[hugomrdias]

We can make our bundle the one used when loaded using UNPKG with a script tag, include all the formats easily using .aegir.js to add an alias for webpack.

I think that's a good compromise - can we do this please @hugomrdias?

Yes

To confirm:

• In Node.js require('ipfs')

  • all IPLD formats included

• In browser application bundle require('ipfs') bundled with webpack/browserify/etc.

  • only ipld-dag-pb, ipld-dag-cbor and ipld-raw included

• CDN bundle <script src="https://unpkg.com/ipfs/dist/index.min.js"></script>

  • all IPLD formats included

Yes, correct.

[alanshaw]

@hugomrdias I've rebased for green CI

[daviddias]

Commenting now upon request

Folks, this seems like a good win when it comes to bundle size. Did you get the chance to make sure that the experience of https://github.com/ipfs/js-ipfs/tree/master/examples/traverse-ipld-graphs && https://github.com/ipfs/js-ipfs/tree/master/examples/explore-ethereum-blockchain is still flawless?

Also, I see that no tests were added to check the remaining formats integration. Was this intentional?

[alanshaw]

Did you get the chance to make sure that the experience of https://github.com/ipfs/js-ipfs/tree/master/examples/traverse-ipld-graphs && https://github.com/ipfs/js-ipfs/tree/master/examples/explore-ethereum-blockchain is still flawless?

Since these are Node.js based examples they should be fine - in Node.js all the formats are still included (as always). This would also have been checked as part of the release process for 0.35.

Also, I see that no tests were added to check the remaining formats integration. Was this intentional?

No that would be a good test to add. We don't do any testing with the built CDN bundle and we really should.

[hugomrdias]

@alanshaw our karma tests, effectively test our "built CDN bundle" just doesn't run terserjs to increase speed. There is an issue in aegir to add terserjs to karma to be 100% the same thing.