Keystone-classic: Fields pull in all of lodash

[josephg]

var _ = require('lodash');

Lodash is _huge_ (520k), and we're only using a tiny part of it. The field types shouldn't be pulling in the entire lodash library. (Its smaller if its minified, but we aren't minifying it - see #2825).

[josephg]

Oh, and its being pulled in twice - once for fields.js and another time for index.js (which pulls in all the code in fields.js in turn)

browserify --full-paths  -t babelify admin/client/index.js -x async -x blacklist -x bytes -x classnames -x color -x display-name -x elemental -x expression-match -x history -x i -x list-to-array  -x marked -x moment -x numeral -x qs -x react-addons-css-transition-group -x react-alt-text -x @jedwatson/react-color -x react -x react-dom -x xhr -x vkey -x react-select | discify --open

This is index.js - notice it includes both lodash (huge) and all of the fields again (?)

[mxstbr]

Could be fixed in two ways:

1. Require the specific functions we need instead of all of lodash with require('lodash/mapValues').
2. If we don't use a lot of lodash, replace it with homegrown utility functions for the few uses we have.

[morenoh149]

@josephg what code viz tool is that? 👀
@mxstbr I like lodash but for a tool we want others to use in their stack we should probably be more disciplined. Requiring the specific functions, option 1, is fairly straightforward.

[mxstbr]

Absolutely agreed!

[morenoh149]

TIL webpack 2 will have treeshaking https://medium.com/modus-create-front-end-development/webpack-2-tree-shaking-configuration-9f1de90f3233#.wkswzd9ng which would only pull in what's used

[josephg]

@morenoh149 disc

Yeah, most of the uses of lodash aren't needed in a modern JS environment. (Eg Object.assign > _.defaults, etc). But keystone still supports node 0.12, which doesn't support most of the ES2015 features. So we can't use real javascript there.

I took a look through the lodash uses and there's a few weird uses too - like the use of a deep equality check in HtmlField and RelationshipType. (In my experience needing a deep equality check is usually code smell. For big objects its slow, and for small objects you should be able to use static references.)

[creynders]

I'm all in favour of dropping node v0.12 support in KS v0.4 ... Active LTS ran till April and any software <1.0 is expected to move forward fast IMO

[mxstbr]

Me too, but @JedWatson wants to keep v0.12 support for [email protected] and then start breaking things and moving to supporting v4.0+.

[JedWatson]

We've spent enough effort keeping 0.4 compatible with 0.12 that at this point, I don't see it as being relatively much of an issue to leave things as they are for that release. As soon as that happens and it's stable, we'll be doing a big round cleanup w/ some breaking changes for 0.5, and I'm all for going to Node 4+ then.

Realistically Keystone should have been called 1.0.0 when 0.3 was released, my bad that we didn't do that but we should think of it as having that level of stability. Let's use the next few minors to get through some big stuff we've got queued quickly, and hit 1.0.0 as soon as we've actually thought through and published APIs for stuff we'd like to support for the next year (or whatever time period makes sense)

I've added lodash to our common scripts bundle, which means it'll now be excluded from all the other client side builds (I thought it was there, but must have missed it)

That has the effect of removing the complete package from any bundle, _unless_ we use explicit imports like require('lodash/mapValues') (that will actually currently have the effect of bundling _more_ client side code if we do that, because it won't be recognised as a package to exclude)

[JedWatson]

(to be clear, that means I'm closing this because it should be fixed!)

It also speeds up our runtime bundle build too :)

[wmertens]

We should use https://github.com/lodash/babel-plugin-lodash, which automatically imports only the babel modules that are actually used.

Then there is also the webpack plugin, which can make an even tighter build by excluding features like syntactic sugar.

[josephg]

Or once 0.4 launches just take it out. You don't need lodash with es6.