ipfs/js-ipfs

Bundling js-ipfs and js-ipfs-api for the Browser

dignifiedquire opened this issue · 16 comments

dignifiedquire commented

JS IPFS is a large collection of modules that aim to implement IPFS in Node.js and the browser. As such the distributions of these modules has a specific set of constraints.

Our current setup is not bad, and does generate bundles usable in Node.js and the browser, but there are some pain points that need work.

Current Pain Points

  1. Bundles are quite large
  2. Lots of dependencies are duplicated, for example readable-stream is included 12 times in the current js-ipfs browser bundle.
  3. Developers have to know very domain specific configurations to
    be able to use browserify or webpack.
  4. We break browserify and webpack compat without knowing about it
    until we get a bug report.

Optimization Goals

  1. Bundle Size
  2. Ease of use for developers embedding the library (i.e. Orbit)
  3. Ease of use for contributors

Module Formats

There are two different module formats for JavaScript modules in main use today.

  1. CommonJS
    • var dep = require('dependency')
    • Only native format in Node.js at the moment.
  2. ES2015 Modules

The current code base uses CommonJS.

Available Tooling

The tooling landscape is quite large today, with things developing and changing quite rapidly. The for us currently relevant tooling is listed below.

Module Bundlers

A module bundler can take in many JavaScript files and generate a bundle, which is usable in the browser.

  1. [Webpack](CommonJS, ES2015)
  2. [jspm](CommonJS, ES2015)
  3. [Closure Compiler](CommonJS, ES2015)
  4. [Rollup](CommonJS, ES2015)
  5. Browserify
  6. [Babel](CommonJS, ES2015)

ES2015 Transpilers

Transpilers can transform code written with ES2015 features and output code that is usable in ES5 (and lower) environments.

  1. Babel
  2. Typescript
  3. Closure Compiler

A good comparision of the differences in size and runtime can be found in The cost of transpiling ES2015 in 2016.

Proposal

Given the set of constraints mentioned above, the following is a list of steps I suggest to improve and solve our current pain points.

1. Improve build artifacts

Similar to what PouchDB does, the end result for Node.js and the browser should be a single file.

If there are differences between Node.js and browser, modules use two different entry points

  • src/index.js - Original source for node.js
  • src/index-browser.js - Original source the browser

For the builds we target the same places as currently

  • dist/index.js ES5 code for the browser
  • lib/index.js - ES5 code for node.js

but lib/index.js will be a single file, fully transformed rather than still many files such that treeshaking and processing of things like webpack loaders already happend and this is runnable through in node.js directly.

To make tooling aware of what is avaliable, the following should fields should be in package.json

"main": "./lib/index.js",
"jsnext:main": "./src/index.js",
"browser": {
  "./lib/index.js": "./dist/index.js"
},
"jspm": {
  "main": "dist/index.js"
}
Benefits
  • Fully compatabile out of the box, with default configuaration with
    • browserify
    • webpack
    • jspm
    • rollup
Drawbacks
  • Transpiled code in lib/index.js is a bit harder to read as it
    is now a single large file.

2. Test webpack & browserify in CI

  1. Build with the default configurations for browserify and webpack.
  2. Run the full test suite against these versions.
Benefits
  • We can be sure that our builds are usable by other developers.
Drawbacks
  • CI run time increases.

3. Move to ES2015 Modules

  • Using tools like cjs-to-es6 this is pretty straight forward for our own modules.
  • For dependencies that do not yet publish a build which uses ES2015
  • Enable tree shaking in our webpack build.

Benefits

  • Smaller module size, due to the availability of statically analyzable dependencies and so allowing us to use tree shaking

Drawbacks

  • Not runnable in Node.js directly anymore until they integrate ES2015 modules or you use something like babel-register.

4. Carefully audit the dependency tree

  • Look at all of them
  • Migrate where needed and large enough benefits are clear to ES2105 modules
  • Major culprits that we know about
    • all shims for Node.js functionality in the browser
    • forge
    • web-crypto -> browserify-crypto
    • readable-stream and all users of it
Benefits
  • Only include what we absolutly need
  • Improves tree shaking if we can use dependencies that use ES2015 modules.
Drawbacks
  • Takes time

Resources

Blog Posts

Issues on IPFS

jbenet commented

Prob best to keep discussion here :)
On Sun, Aug 21, 2016 at 10:28 Friedel Ziegelmayer notifications@github.com
wrote:

Closed #429 #429.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#429 (comment), or mute the
thread
https://github.com/notifications/unsubscribe-auth/AAIcocxYGBG8xROmZAivJ0ISEUZJOVmOks5qiGCPgaJpZM4JpTlo
.

dignifiedquire commented

Moving discussion back here.

Re comments from @jbenet in the other issue

This type of optimization (removing code) should not be so complex. And I'm not convinced it has to be.

I am very happy to see simpler solutions that achieve the same level of gains, so far I have not seen one.

Not being compatible with node is not an option.

We are still node compatible, but would have a compilation step before hand, as we have for the browser or we have in go.

Using ES6 imports should be a last resort. They demonstrate very poor judgment in design and are a huge step backwards for code readability, and simplicity (in the ritch hickey sense of the word). It also runs counter to the programming model of ipfs. (Immutable dags) and is not something WE should promote and lead to, because it is going to make future code harder to use. (This is a long term tussle.)

There is a lot of personal opinion in here, and I don't want to make this discussion about the design decisions made in ES2015 modules. Fact is that they allow deterministic static analysis by design, something which CommonJS, due to its dynamic nature not fully allows.

optimizations should not require all this-- start by removing unused stuff. Look at the way people do it now-- proper static analysis.

What people do is moving to ES2015 modules because at the current tooling state only Closure Compiler is able to remove the static part of unused CommonJS modules. All other tooling that I tested it not able to do this entirely.

Why can't you throw googleclosure at it? That's a much more rigorous solution that removes all dead code, optimizes heavily. I seem to recall it reduces equal functions (or was going to). (It's not tree shaking, it's dagify, picking the only leafs you need, and compress)

We can, and we can't. Using Closure Compiler, with advanced optimizations (the only level that does give us these benefits) comes with a set of pretty hard draw backs.

  • ALL code has to adhere to these limitations which many are not followed by modules on npm. This in turn means we have to audit all dependencies to ensure we don't accidentally break the compiler and our code.
  • We introduce Java into our build pipeline

there seem to be a lot dependent thinking here, meaning there seems to be a very complicated path because of decision dependencies that are not made explicit here. I'm not confident that this is actually necessary, and I would really like to see a walk through of those decisions before claims like "we have to move to ES6 imports because we have to use webpack tree shaking because we have to use webpack" can be validated.

I don't see this. Please read the details above again. We don't need to use webpack, but we also don't want to write the whole pipeline ourselves.

Webpack has served as well in the past and continues to improve. For tree shaking there are multiple solutions and pipelines available, of which we could choose either with it's pros and cons. You can refer to the above mentioned The cost of transpiling ES2015 in 2016 to get a comparison of the different pipelines.

dignifiedquire commented

I found another really good tool to analyse where the size is left. https://github.com/danvk/source-map-explorer will allow you to actually look into the minified version and with the help of source maps figure out where the code comes from.

dignifiedquire commented

After more research I think I have to come to a better solution, that is less radical.

  • Stay with CommonJS until ES Modules are supported in Node.js
  • Stop transpiling, we are not supporting old browsers any way, if you look at https://kangax.github.io/compat-table/es6/ you can see that as long as we stay within the feature range of Node.js 6 we actually don't need to transpile and ship polyfills into the browser. This means,
    • no more lib folder, just pointing directly to src for npm
    • dist/index.js will be a full version, but not transpiled
    • dist/index.min.js, this is an open question, uglify does not support minification of ES 6, there is a harmony branch and there is an alternative called https://github.com/babel/babili
    • No more PhantomJS tests, it doesn't work with webcrypto anyway
  • We need to communicate clearly what we are doing, and get cross browser tests with a chart from SauceLabs on each readme so users know what to expect.
  • Make sure we test with a built file, instead of a specific version for tests.
  • 👍3
  • 🎉2
dignifiedquire commented

Also dropping Node 4 support, given that 6 is the new LTS version starting next month.

dignifiedquire commented

Also I should mention, part of this effort will be ensuring browserify, rollup and webpack@2 work out of the box, or with only a minimal configuration.

pelle commented

@dignifiedquire I would like to recommend that you continue transpiring it. The most important browsers that would need it are mobile browsers. You could create 2 different versions though.

Since before Devcon2 I've been busy working on other things, but I'm going to start reviewing this again to see if we can start using the ipfs-js libraries in our browser/RN code for uPort.

The largest problem unless there have been many changes since I last looked at it, is the node first approach. I would really recommend using browser http abstractions as the core, such as fetch or XHR that are built into the browser. There are node implementations of then. fetch in particular is a modern promise based way of doing http.

Remember that for node it doesn't matter that you're pulling in a few extra dependencies. But for the browser it does.

dignifiedquire commented

@pelle in what situations would you use our transpiled version over a bundle that you would generate yourself and transpile?

pelle commented

@dignifiedquire The only function of a minimized dist file is for them to be included directly on a web page. Ideally you would CDN them somewhere.

Large dependencies in production web apps will often be loaded separately to allow them to be cached by the browser and not reloaded if a minor app change happens.

For the case of bundling them myself I don't need the dist files. I'll require the dependencies as they are and bundle my whole app.

dignifiedquire commented

right but if you do seperate bundles in production you really need to generate them yourself. The current use case why we provide the dist files is for quick start and demos, both of which are usually not too concerned with strict broser compat. So I still think we should stop providing transpilled builds and only provide imple receipes to get a transpilliatiom pipeline setup for those that need one.

pelle commented

If thats the only use case then thats fine though. I'm much more concerned with lowering the size of dependences on.

dignifiedquire commented

First set of changes is up here: ipfs/aegir#65 I will use this as a baseline to make sure that modules are being built and working with a minimal config of just using babel + json loader in webpack. Which translates to babelify + browserify with no default config.

daviddias commented

Relevant ipfs-inactive/js-ipfs-http-client#416 as it right now looks kind of scary on install

  • 👍1
dignifiedquire commented

This was shipped in #485 :) so closing

daviddias commented

@dignifiedquire the cherry on top of this cake would be to have two quick start tutorials of how to bundle js-ipfs and js-ipfs-api with browsersify and webpack.