Huge component size #112

cristijora · 2017-02-12T23:59:24Z

Just noticed that the overall component has 400+ kb in size.
I must say that's pretty huge for a component that generates fields.
After taking a look at the source code I noticed that moment.js is used in several places for very basic functionalities. I think you can get rid of it to save some space.

Only by removing all moment js dependencies (4) the size shrinked from 400kb to 176kb.
There is also one component fieldVueMultiSelect which for some reason uses the whole Vue library. I don't think there is need to bring the whole library just to instantiate that component. There should be another way of doing it. By removing the vue dep the final file further reduces to 126kb.

Taking down lodash util function brings the file down under 100kb.
To sum it up, by removing only external dependencies (moment, vue and lodash) I managed to bring the library down from 400+ kb to 55kb and by removing external components it goes down to 29kb.

The size here is huge factor for people deciding whether to use such a component or not. I would say we could shrink this one down to 100-150kb very easily and then with some effort it can be brought under 100kb.

The text was updated successfully, but these errors were encountered:

icebob · 2017-02-13T08:18:27Z

Thank you for your explain. Could you make a PR with this changes?

cristijora · 2017-02-13T08:27:56Z

Yes, I will make one today with some simple changes which should reduce the size in half :)

icebob · 2017-02-13T08:28:09Z

This is the current bundle stat:

The source is only 2.8%. The others: vue (28%), moment (35%), lodash (42%)

cristijora · 2017-02-13T08:31:36Z

Interesting. What did you use to get those stats?

These are the 2 files you get when installing the component via npm (402 kb)

icebob · 2017-02-13T08:36:37Z

I used this tool: https://chrisbateman.github.io/webpack-visualizer/
If you make a build, there will be a stats.json in root of repo. And you need to drop this file to the visualizer site.

lionel-bijaoui · 2017-02-13T09:43:06Z

Hi, this issue has the same goal as my other one #67
I absolutely agree with @cristijora and at the time, I already got impressive result.

I think the generator should be on it's own and people should just pick and choose which one they want.
I don't know if each field should have it's own sub repo (eg. vfg-checkbox) or if we could provide the same flexibility with only one repo.

What are your thought ?

cristijora · 2017-02-13T09:54:02Z

Agree witht that @lionel-bijaoui . What's the impact of externals ? Don't quite understand it.

lionel-bijaoui · 2017-02-13T10:43:57Z

@cristijora with external, you need to add them globally (via a script tag) and webpack will use that reference. But it was a solution that fitted my need (my project use moment.js anyway), and I'm not sure this is the best for vfg.

At the time I envisioned that you could specify by field, what dependency it have and simply add it in a script tag (some fields do that). But I'm not sure that it would work if you import the lib. Also it force to use the lib in a global context.

Doc : https://webpack.github.io/docs/library-and-externals.html

Edit: Better doc (for webpack 2 but it is mainly the same process) : https://webpack.js.org/guides/author-libraries/

cristijora · 2017-02-13T11:09:57Z

Ah got that. Don't think that's a robust option. Ideally it would be nice to get rid of the external dependencies or use strictly what's necessary which is ok in the case of lodash, but for some reasons it imports a lot of code. Moment is used only in 4 places for some basic stuff. We can get rid of it I think.

Use moment.min to reduce build size

#112 Get rid of Vue dependency in the code.

icebob · 2017-02-13T12:54:57Z

Thanks @cristijora . Your PR merged. Now there is only lodash (75% of bundle).

icebob · 2017-02-13T12:56:44Z

Maybe webpack 2 tree-shaking can be remove the unused lodash functions?

cristijora · 2017-02-13T13:25:29Z

Could be an option. Will give it a read.

lionel-bijaoui · 2017-02-13T14:03:34Z

I would like we avoid changing webpack version for now.
There is actually an option to do that for webpack 1 : https://github.com/lodash/lodash-webpack-plugin with https://www.npmjs.com/package/babel-plugin-lodash

Also, can we make a decision about "modularizing" fields ?

Use babel-plugin-lodash to support "tree-shaking" for lodash

icebob · 2017-02-14T08:31:18Z

@lionel-bijaoui I don't support modularizing fields, because the size of code of all fields is small. ~ 10kb. So we can decrease the size with modularizing. We need to solve the momentjs & lodash dependencies instead.

lionel-bijaoui · 2017-02-14T10:02:24Z

Well, moment and lodash are priority, but since vuejs 2 compiled sit at about 12k, 10k is actually big. Vfg could keep some super basic field (classic html5 inputs). But all the others (jQuery based field, others like vuemultiselect, etc.) can use the same system as the custom fields and be use if needed. It would allow anyone to create fields for the system with a naming convention (vfg-***) like babel did. At least, unused field should not be bundled in the final file. For example, I could create some sort of "config" for vfg to tell it that I explicitly use this and that field (maybe with `Vue.use()`) and webpack could throw away unused fields at bundling. Sorry to insist on that point but for my project I need a lot of custom fields and every byte saved is welcomed. I would like to avoid to create a fork only for performance reasons. Thank you for your understanding. Le 14 févr. 2017 09:31, "Icebob" <[email protected]> a écrit :

…

@lionel-bijaoui <https://github.com/lionel-bijaoui> I don't support modularizing fields, because the size of code of all fields is small. ~ 10kb. So we can decrease the size with modularizing. We need to solve the momentjs & lodash dependencies instead. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADT81IsmEDfBI9n_6DNlpewzL8akcMycks5rcWZWgaJpZM4L-rOE> .

cristijora · 2017-02-14T10:12:26Z

i actually partially agree with @lionel-bijaoui here. This would open some possibilities like adding some vue based components while not affecting the size of the whole thing. I don't have a good solution for it though. Maybe have something similar to the custom field implementation. Simply import the necessary external lib and declare it globally ? Ex:

import fieldMultiSelect from "vue-form-generator/src/fieldVueMultiSelect.vue"
Vue.component("fieldMultiSelect", fieldMultiSelect);

This would register the component as an abstract field and it will work since the component meets the requirements of the abstract field custom inputs.

icebob · 2017-02-14T10:51:55Z

In this case we can create two bundles. A minimal version (for only html5 field, but can be import the further fields) and a full version (as current bundle, which contains every built-in fields)

Opinion?

lionel-bijaoui · 2017-02-14T10:53:38Z

@icebob That was exactly my initial proposal #67 and I would be very happy with it 😄

icebob · 2017-02-14T10:54:59Z

Sorry, I didn't remember :)

lionel-bijaoui · 2017-02-14T10:55:30Z

Ahah, no problem !

cristijora · 2017-02-14T11:48:01Z

Agree. Seems like a simple approach. If the user doesn't care about size he just imports the default full version but if he does care then he imports the minimal version and declares the components himself.
I think we would need to add something similar to what I did in my #112 PR to each component that wouldn't be implicitly included in the minimal build.

if(!this.$root.$options.components["multiselect"])

Example:

if(!this.$root.$options.components["fieldDateTimePicker"])

icebob · 2017-02-14T12:03:29Z

And need to rewrite the dynamic field loader part

cristijora · 2017-02-14T12:05:06Z

Yeah. A simple option would be to place the optional fields in a separate folder 😄

icebob · 2017-02-14T12:06:15Z

Nice suggestion :)

icebob · 2017-02-14T12:13:29Z

So the tasks:

kill unused lodash codes with babel-plugin-lodash or lodash-webpack-plugin
retire momentjs and use date-fns or fecha
separate bundle to
- core/base version which contains only html5 input + some fields
- full/complete version which contains all built-in fields (current bundle)
- separate field vue files to two directory
- rewrite dynamic field loader code
- modify webpack config to create two bundles

cristijora · 2017-02-14T12:26:15Z

The first 2 are in this PR
The second one needs adjustments because of a format error though.
The first task is done in the PR. (babel-plugin-lodash used)

icebob · 2017-02-14T12:28:40Z

And babel-plugin-lodash working properly?

cristijora · 2017-02-14T12:36:00Z

Yes it works fine. Reduced like 75kb with that only.
Before

After

icebob · 2017-02-14T12:38:59Z

Great! 🎉

lionel-bijaoui · 2017-02-14T12:49:01Z

I can help with the webpack config if needed.
Not sure what you mean by "separate field vue files to two directory".

cristijora · 2017-02-14T12:58:13Z

@lionel-bijaoui
This code basically looks in the fields folder and registers the components

let Fields = require.context("./fields/", false, /^\.\/field([\w-_]+)\.vue$/);
	let fieldComponents = {};
	each(Fields.keys(), (key) => {
		let compName = key.replace(/^\.\//, "").replace(/\.vue/, "");
		fieldComponents[compName] = Fields(key);
	});

//some other code
components: fieldComponents,

A very simple approach here would be to put the optional components which we don't want to include in a separate folder and have some webpack config at this point
let Fields = require.context("./fields/", false, /^\.\/field([\w-_]+)\.vue$/);
where we can replace ./fields/ with some array and have 2 configurations for the default build

foldersToInclude = ["./fields/","./optional"];

and for the minimal build

foldersToInclude = ["./fields"]

Instead of
let Fields = require.context("./fields/", false, /^\.\/field([\w-_]+)\.vue$/);
we would iterate the the array from above and add the components.

icebob · 2017-02-14T13:05:28Z

And you can use DefinePlugin and declare a variable in webpack config e.g. FULL_BUNDLE. And in the code you can use it

if (FULL_BUNDLE) {
  foldersToInclude = ["./fields/","./optional"];
} else {
  foldersToInclude = ["./fields"]
}

Maybe :)

lionel-bijaoui · 2017-02-14T13:15:08Z

@cristijora thanks a lot for your help. You deserve the collaborator title more than I do 🥇
@icebob Ok I will do a PR when it is ready. I didn't know about DefinePlugin but it could be very helpfull for my own project too. Thanks !

icebob · 2017-02-15T08:29:20Z

@lionel-bijaoui I would like to discuss that we marked some fields as "deprecated", because we have a common "input" field. Are we remove them in v2?

lionel-bijaoui · 2017-02-15T08:34:56Z

Yes, that would be a good time to do it. Most of them are duplicate of the new input based field. Let me double check the list to make sure they are no false positive. Le 15 févr. 2017 9:29 AM, "Icebob" <[email protected]> a écrit :

…

@lionel-bijaoui <https://github.com/lionel-bijaoui> I would like to discuss that we marked some fields as "deprecated", because we have a common "input" field. Are we remove them in v2? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#112 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADT81Gq_DUH-ibPKNaQnK6JYDviy2FzBks5rcrdhgaJpZM4L-rOE> .

icebob · 2017-02-15T08:47:25Z

OK, new issue for this: #120

icebob · 2017-02-15T11:17:59Z

Full bundle: 75k
Core bundle: 39k

Great!

cristijora · 2017-02-15T11:57:49Z

Awesome. Nice to see going it down from 400+kb to 40kb.

icebob added difficulty: medium enhancement labels Feb 13, 2017

cristijora pushed a commit to cristijora/vue-form-generator that referenced this issue Feb 13, 2017

vue-generators#112 Get rid of Vue dependency in the code.

01fa81b

Use moment.min to reduce build size

icebob added a commit that referenced this issue Feb 13, 2017

Merge pull request #113 from cristijora/improve-bundle-size

98966df

#112 Get rid of Vue dependency in the code.

lionel-bijaoui modified the milestone: v2.0.0 Feb 13, 2017

cristijora pushed a commit to cristijora/vue-form-generator that referenced this issue Feb 13, 2017

vue-generators#112 Get rid of moment over date-fns

282f2f9

Use babel-plugin-lodash to support "tree-shaking" for lodash

icebob mentioned this issue Feb 14, 2017

Moment -> fecha #117

Merged

icebob mentioned this issue Feb 15, 2017

Even more modular #67

Closed

lionel-bijaoui mentioned this issue Feb 15, 2017

Separate bundle and remove deprecated fields #121

Merged

icebob closed this as completed Mar 1, 2017

Huge component size #112

Huge component size #112

Comments

cristijora commented Feb 12, 2017 • edited Loading

icebob commented Feb 13, 2017

cristijora commented Feb 13, 2017

icebob commented Feb 13, 2017

cristijora commented Feb 13, 2017

icebob commented Feb 13, 2017

lionel-bijaoui commented Feb 13, 2017

cristijora commented Feb 13, 2017

lionel-bijaoui commented Feb 13, 2017 • edited Loading

cristijora commented Feb 13, 2017

icebob commented Feb 13, 2017 • edited Loading

icebob commented Feb 13, 2017

cristijora commented Feb 13, 2017

lionel-bijaoui commented Feb 13, 2017 • edited Loading

icebob commented Feb 14, 2017

lionel-bijaoui commented Feb 14, 2017 via email

cristijora commented Feb 14, 2017 • edited Loading

icebob commented Feb 14, 2017

lionel-bijaoui commented Feb 14, 2017

icebob commented Feb 14, 2017

lionel-bijaoui commented Feb 14, 2017

cristijora commented Feb 14, 2017

icebob commented Feb 14, 2017

cristijora commented Feb 14, 2017 • edited Loading

icebob commented Feb 14, 2017

icebob commented Feb 14, 2017 • edited by lionel-bijaoui Loading

cristijora commented Feb 14, 2017

icebob commented Feb 14, 2017

cristijora commented Feb 14, 2017

icebob commented Feb 14, 2017

lionel-bijaoui commented Feb 14, 2017 • edited Loading

cristijora commented Feb 14, 2017 • edited Loading

icebob commented Feb 14, 2017

lionel-bijaoui commented Feb 14, 2017

icebob commented Feb 15, 2017

lionel-bijaoui commented Feb 15, 2017 via email

icebob commented Feb 15, 2017

icebob commented Feb 15, 2017

cristijora commented Feb 15, 2017

cristijora commented Feb 12, 2017 •

edited

Loading

lionel-bijaoui commented Feb 13, 2017 •

edited

Loading

icebob commented Feb 13, 2017 •

edited

Loading

lionel-bijaoui commented Feb 13, 2017 •

edited

Loading

cristijora commented Feb 14, 2017 •

edited

Loading

cristijora commented Feb 14, 2017 •

edited

Loading

icebob commented Feb 14, 2017 •

edited by lionel-bijaoui

Loading

lionel-bijaoui commented Feb 14, 2017 •

edited

Loading

cristijora commented Feb 14, 2017 •

edited

Loading