jxnblk / rmdi

React Material Design Icons – built with Pixo, Styled Components, and Styled System
https://jxnblk.com/rmdi
MIT License
129 stars 11 forks source link

Tree-shaking not possible #1

Closed Andarist closed 6 years ago

Andarist commented 6 years ago

By reading:

// import icons individually for better tree-shaking

I assume you care about tree-shaking. Based on that I'd like to report serious problems with this library's structure that prevents tree-shaking completely.

Let's examine what happens when we bundle a simple file:

import { Accessibility } from 'rmdi'
console.log(Accessibility)

with following config:

const webpack = require('webpack');
const path = require('path');

module.exports = {
  mode: 'production',
  entry: './src/index.js',
  output: {
    path: path.resolve(__dirname, "dist"),
    filename: './index-rmdi.js',
  },
  externals: {
    react:'react',
    'react-dom':'reactDom',
    'styled-system':'styledSystem',
    'styled-components':'styledComponents',
  },
}

We can see by inspecting the output file that nothing really got removed (tree-shaken) out of the bundle and it weighs 100kb minified & gzipped:

> gzip -c dist/index-rmdi.js | wc -c
102580

There are several things that could improve the situation:

RIP21 commented 6 years ago

Amazing issue :) But I assume it's partially can be solved via direct import like import Accessible from 'rmdi/lib/Accessible' Didn't test it tho, but maybe a nice workaround for kinda manual tree shaking in the current state of the lib.

@Andarist BTW, if you are that familiar with all that, will be a cool thing to do is to design "tree-shaking cheatsheet" or something :) In FIgma/Sketch or whatever. Just MD file should work too tho :) Will be more than happy to apply all those practices to some of the internal projects I'm working on :)

Andarist commented 6 years ago

But I assume it's partially can be solved via direct import like

It indeed can be solved with direct imports, but that still leaves us with a problem that not everyone will do that. IMHO it also kinda leaks internal directory structure to the user (/lib/).

Also it won't solve issue of "transitive dependencies" and tree-shaking in general. This solution assumes that all of your files are used so their deps (rmdi in this case) are used too. IMHO with fast changing projects, code-splitting etc it's quite easy to end up with the situation where your file is only partially used (maybe you are trying out some feature, new layout, whatever) and current setup wouldn't allow other popular tools to remove possibly dead directly imported icon.

@Andarist BTW, if you are that familiar with all that, will be a cool thing to do is to design "tree-shaking cheatsheet" or something :)

I've kinda started it with my guide - https://developers.livechatinc.com/blog/how-to-create-javascript-libraries-in-2018-part-1/ . Second part presents various techniques how you can make your libraries smaller. 3rd part (that I will write some day 😉) will be based on already linked issue and some other github comments of mine such as this one.

jxnblk commented 6 years ago

Thanks! I haven’t finished all the setup I’d like to add to this repo yet and was looking for some help with some of the things you’ve mentioned above. I’ll probably open a few other related issues with clearly stated actionable items, such as the sideEffects: false field, some bundle size tests, etc

Andarist commented 6 years ago

I can work on some of those later

jxnblk commented 6 years ago

Going to close this issue out in favor of more clearly stated ones