Update "Optimizing bundle size" README section

[istarkov]

Since we have es2015 modules support, current size optimization suggestion is outdated and gives not the best result.

[istarkov]

cc @TrySound

[codecov-io]

Codecov Report

Merging #367 into master will decrease coverage by 2.27%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #367      +/-   ##
==========================================
- Coverage   92.98%   90.71%   -2.28%     
==========================================
  Files          51       51              
  Lines         328      474     +146     
==========================================
+ Hits          305      430     +125     
- Misses         23       44      +21

Impacted Files	Coverage Δ
src/packages/recompose/withProps.js	85.71% <0%> (-14.29%)	⬇️
src/packages/recompose/renameProp.js	85.71% <0%> (-14.29%)	⬇️
src/packages/recompose/mapProps.js	85.71% <0%> (-14.29%)	⬇️
src/packages/recompose/pure.js	85.71% <0%> (-14.29%)	⬇️
src/packages/recompose/defaultProps.js	87.5% <0%> (-12.5%)	⬇️
src/packages/recompose/flattenProp.js	87.5% <0%> (-12.5%)	⬇️
src/packages/recompose/onlyUpdateForKeys.js	88.88% <0%> (-11.12%)	⬇️
src/packages/recompose/shouldUpdate.js	90% <0%> (-10%)	⬇️
src/packages/recompose/getContext.js	90.9% <0%> (-9.1%)	⬇️
src/packages/recompose/withContext.js	92.3% <0%> (-7.7%)	⬇️
... and 18 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52e3baf...57b08f5. Read the comment docs.

[TrySound]

Looks good.

[gregberge]

Tree shaking isn't working very well, especially with recompose. Every export makes a call to a function (createHelper). Your implementation is good but it will sadly not impact the final build size of users.

I suggest you to replace the content of the section with the lodash approach like I do in recompact.

[TrySound]

Thanks for your mention. There's a solution. Just to prevent createHelper.

But what is the purpose of your library? Performance? It's our purpose too. A few new utils? It can be different library without copying stuff.

[gregberge]

@TrySound yes we have to do it, but there is a few issues, tree shaking isn't very reliable for now.

We use a lot of composition in React at Doctolib, we used recompose for a long time and contributed several times. Recompose was not updated during 6 months, the project was like abandoned. Also in the same time we encountered some performance issues and a lot of debug pain especially in the tree view. After one year of work on observables and with connectObs and withObs, I decided to create recompact to have the control on the core of our stack and being able to give it the right direction. I figured out that @acdlite had the same idea. That's how recompact was born.

After the release of recompact, some releases of recompose appeared and it sounds like going in the good direction now. We could consider merging the two projects at some time. On my side, I have a lot of ideas and improvements for the future.

The purpose of recompact is about performances and debug. Using observables under the hood we can avoid creating some React components in the majority of cases.

[TrySound]

Fine. Let's start with treeshaking issue. I think it's more improtant to be consistent and have single source of truth.

[istarkov]

The purpose of recompact is about performances and debug. Using observables under the hood we can avoid creating some React components in the majority of cases.

Just curious about real life example of performance benefit (not tests with 10s withState one after each other), as I use recompose a lot and even in projects with thousands of components I haven't seen visually the big difference when I used recompose or plain react classes.

Always I got the same result or both solution are slow, or both works well.

[istarkov]

Wanna add that initially I was also sure that recompose is slow, and it was in some cases - but only in dev mode, when I set NODE_ENV=production it had the (visually) same perf to plain classes as I wrote above

[gregberge]

@istarkov performances is not the first concern, I agree React is very fast and stacking components doesn't introduce a bottle neck in majority of cases. But I like the idea to be able to split my higher-order components into small pieces without thinking about it.

At Doctolib, in our application (similar to Google Agenda), we mount a lot of components and recompact permit us to win a few milliseconds.

[istarkov]

@neoziro got it, thank you for comment about tree shaking, you are right. Seems like I need to read about how tree shaking works, as for now I cant get why call to identity function breaks all the things.

[gregberge]

@istarkov I tested it in a deep way on recompact, also recompose, in fact it's very simple, tree shaking is safe. If a function is executed in the module, it will not drop it because of side effects.

Imagine the code:

export default alert('foo')

Dropping this module will change the behaviour of your application, so it is not dropped. In comparison, in recompose / recompact we do:

export default createHelper(mapProps, 'mapProps')

Tree shaking doesn't know if createHelper create a side effect or not, so the module is included.

There is no simple workaround for recompose / recompact to deal with it (except duplicating code in
createHelper or transpile it before give it to the bundler).

That's why I recommend you to recommend the usage of "babel-plugin-lodash" even if the tree shaking is effective in the project. By the way, good work on the bundle part!

[wuct]

Seems like babel-plugin-lodash still is the best way to reduce bundle size. I've used it for a long time and it works quiet well.

[istarkov]

Removal of createHelper call for production build will make Tree Shaking approach great again.
So let's wait a little.

[wuct]

@istarkov I think we can put both approaches into doc, right?

[evenchange4]

Agree with @wuct 's approach for common building system, and I use babel-plugin-import to do the same thing in my project:

// babel plugin
{
  libraryName: 'recompose',
  libraryDirectory: '/', // default: lib
  camel2DashComponentName: false, // default: true
},

https://github.com/MCS-Lite/mcs-lite/blob/master/packages/babel-preset-mcs-lite/index.js#L45-L49

[istarkov]

@wuct I'm not sure, having both you need to support both ;-)

[TrySound]

Having both is the same as lodash entry points problem. Somebody like lodash/module, somebody lodash.module, somebody import { module } from 'lodash';

[wuct]

I've reopened this issue accidentally 😓

[wuct]

Let's try removing all createHelper and see if it works. I agree we should put only the best practice in the doc.

[TrySound]

@istarkov You may reopen this PR now. Tree shaking works well.