magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.52k stars 9.31k forks source link

Javascript Bundling produces huge 13MB js files #4506

Closed VegasJames closed 7 years ago

VegasJames commented 8 years ago

Steps to reproduce

  1. Install Magento via composer. v2.0.5
  2. Use admin to enable Javascript Bundling. (Stores -> Configuration -> Advanced -> Developer -> JavaScript Settings) and set Merge JavaScript Files and Enable Javascript Bundling to Yes
  3. Left "Minify JavaScript Files" to No so the file could be inspected, but even with it as Yes the bundle0.min.js files is 8MB.
  4. Had to run bin/magento setup:di:compile
  5. Then did bin/magento setup:static-content:deploy
  6. Observe the size of the generated bundle0.js file.

    Expected result

Before bundling Chrome showed about 515KB of javascript being loaded, bundling the files in to one should result in a file of a similar size, not 25 times as big.

Actual result ls -lh pub/static/frontend/Magento/blank/en_US/js/bundle/ total 13M -rw-rw-rw-. 1 magento apache 13M May 14 07:23 bundle0.js

I've investigated the content of the file somewhat. After a lot of valid javascript we get to this bit: //# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJz (truncated) This goes on and on with random characters for 1,414,372 bytes. The rest of the file then appears to be valid javascript, so unfortunately the above base64 only explains about 10% of the bloat, so I'm not sure where the rest comes from, but certainly this seems wrong.

Additionally, this is more of a feature request, but can the bundling leave out jquery and jquery-ui? These are the two largest files accounting for 700KB of javascript and almost 200KB of data transferred, and they also could be fetched from Google CDN instead: https://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js https://ajax.googleapis.com/ajax/libs/jqueryui/1.10.4/jquery-ui.min.js

pboisvert commented 8 years ago

Chuck--can you get the team to investigate given the potential for performance impact if reproducible?

rudik99 commented 8 years ago

I also suffered the same problem on the same version installed via composer.

viktor-zhuromskyy commented 8 years ago

Same thing here. :-1:

viktor-zhuromskyy commented 8 years ago

Not even that! On 2.0.5 after all static content is compiled without any errors, there is a fatal error on whole Magento installation dues to errors in DOM bundling. With the JS Bundling OFF everything works normal.

Please see screenshots. image 2016-05-16 17 19 12 image 2016-05-16 17 19 17

choukalos commented 8 years ago

I'll check with the team re bundle size. I believe we implemented a very simple bundling algorithm so it's possible we're pulling in some additional files in the bundle.

Is anyone else seeing bundle.js errors on 2.0.5? @devdesco-ceo did you create a separate Github issue for the bundle error on 2.0.5?

vkorotun commented 8 years ago

@VegasJames It seems like your theme doesn't inherit from Blank or Luma Magento native themes, so exclude list for JS files bundling is missed. You need to configre the exclude list for your custom theme. As a reference example you can have a look into configuration for Blank or Luma theme:

See "exclude" node content.

choukalos commented 8 years ago

@VegasJames Please close or let us know if that didn't address your issue. @devdesco-ceo What themes do you have installed?

viktor-zhuromskyy commented 8 years ago

I have Porto theme installed and my theme is a child theme of Porto which is based on Blank. my /etc/view.xml file has all proper exclude statements for the JS files.

image 2016-05-18 13 02 16

VegasJames commented 8 years ago

incorrect, this is a stock Magento install. the bundle file inside both the blank and Luma theme directories is 13MB

On Wednesday, May 18, 2016, Chuck Choukalos notifications@github.com wrote:

@VegasJames https://github.com/VegasJames Please close or let us know if that didn't address your issue. @devdesco-ceo https://github.com/devdesco-ceo What themes do you have installed?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/magento/magento2/issues/4506#issuecomment-220033699

TommyKolkman commented 8 years ago

I can second VegasJames's problem on 2.0.7. Also, my theme inherits correctly.

atIOCrON commented 8 years ago

@vkorotun we have the same issue verified with Magento 2.0.6 using the default Luma theme:

Without JS Bundling (453.8kb): http://tools.pingdom.com/fpt/#!/czoIzm/https://new.betterbatt.com.au/ With JS Bundling (2.1mb): http://tools.pingdom.com/fpt/#!/c8CxvG/https://new.betterbatt.com.au/

As you can see JS Bundling increases page size by 4x.

I also noticed that JS Bundling when enabled blocks the selection of role resources when adding a custom admin role.

erikhansen commented 8 years ago

From what I've read here and from my independent testing, it seems like the "Enable Javascript Bundling" is currently unusable and should not be recommended for production environments. The way bundling has been implemented for M2 negates much of the benefits gained by RequireJS.

I would recommend the following:

hostep commented 8 years ago

And let me just add one small addition: if your webserver is capable of running HTTP/2 (which will become more and more the case) it is really not recommended to enable bundling. Since HTTP/2 hasn't got performance problems with transferring a lot of smaller files.

southerncomputer commented 8 years ago

Expected '}' File: bef36da44feee702d670095d789f94f7.min.js, Line: 136, Column: 49365

Getting this error on IE and Firefox plus admin section no longer functioning after enabling bundling on 2.0.6 !

This seems about as reliable as the old magento 1 compiler ! Having to load +140 .js files due to this feature not working is severely bad!

erikhansen commented 8 years ago

@southerncomputer Your browser will cache the JS files, so while the first page will load more slowly, subsequent pages will use the cached JS files. And since the JS is being loaded asynchronously via RequireJS, the page will render before the JS files are loaded.

That being said, it does seem a bit much to have ~118 JS files loaded for a vanilla M2 homepage—thus why I'd like to see a smarter bundling feature.

southerncomputer commented 8 years ago

https://github.com/magento/magento2/issues/3040 this is a duplicate ?

erikhansen commented 8 years ago

@southerncomputer No, #3040 is not a duplicate issue of this issue. Although the two issues are somewhat related in that they deal with JS minification/bundling.

However the issue you described in your comment above might be caused by the issue reported in #3040.

southerncomputer commented 8 years ago

https://github.com/magento/magento2/blob/6ea7d2d85cded3fa0fbcf4e7aa0dcd4edbf568a6/lib/web/mage/backend/validation.js

this file throws an error in the bundle regardless if you disable jshrink or not and have bundling enabled. I will try to manually minify this file to see where the file is causing issues in bundling.

The result is in IE11/FF it will throw an unmatched } error on the developer tools console, thus rendering the big bundle unusable for the most part!

Zaylril commented 8 years ago

I have to agree with @erikhansen in that in it's current state the bundling feature is simply not usable. Just think here for a second for a mobile user on 3G, even with the 13meg file gzipped it comes out at a whopping 3.3meg. Add this to the rest of the page and you are looking at 5meg+ for an initial page load. Not great.

Surely a much better solution would be to only bundle files which would be used on every page - or have a config where you could add files you wanted to be bundled together - whether this be automatic or not - meaning you would get some benefit from bundling files for a common.js file and then load the rest as and when they are needed.

Additionally I'm also not sure why the requireJS optimizer is not being implemented - where it combines related scripts together to build multiple bundles which are included as and when they are needed vastly reducing the number of individual files loaded, and greatly increasing performance.

Of course there is a balance to be had between file size and the number of files loaded - but as already mentioned when http2 is implemented this becomes a non-issue, however I can't really see this happening for everyone and all devices in the near future.

I think the only solution is really to create a smarter bundling feature - similar to that of the requireJS optimizer or implementing a build step where smarter bundling occurs. In this way you would reduce the number of requests down for each page and also reduce the file size as well - as was the initial intention of loaded each file separately. This really needs fixing though - especially on the checkout where there is something like 250 JS files loaded - not to mention the .html templates being loaded via AJAX too. Not the best for performance. The worst thing about this also is you can't really implement any kind of server cache on all those JS files to speed it up - as it totally relies on the browser.

erikhansen commented 8 years ago

@Zaylril Thanks for adding to the discussion.

@alankent I'd be curious to hear your thoughts on this issue.

southerncomputer commented 8 years ago

2.1.0-rc2 now creates 5 bundle0.js .. bundle5.js - not that is saving much space but breaks up the files to concurrently load?

erikhansen commented 8 years ago

@southerncomputer Thanks for reporting, but this still doesn't fix the underlying concern that someone visiting a Magento 2 homepage shouldn't be served ~8MB (uncompressed) of JS, most of which is not needed on the homepage.

southerncomputer commented 8 years ago

Well this is odd, After deploying to production mode, I switched to developer mode wiped pub/static/* and now it won't load the bundle0.js pair any more after deploying to production again, it appears to be pulling from pub/static/_cache/merged and loading the signed .min.cs and .min.js files. I've tried wiping var/di var/generation var/view_preprocessed etc. Then deploying using php bin/magento deploy:mode:set production..

The size between the two modes is greatly different as well. Bundle.js was about 8meg uncompressed and the signed .min.css and *.min.js are about 2meg uncompressed?

the bundle0.js .. bundle3.js are located in pub/static/adminhtml/Magento/backend/en_US/js/bundle/ and pub/static/frontend/Magento/luma/en_US/bundle/

How odd is this? I've restarted httpd and php-fpm between the deploy commands to rule out pathcache!

dthampy commented 8 years ago

An internal JIRA ticket -MAGETWO-55587 has been created for the original issue (reported on May 14)

charleskj commented 8 years ago

This issue resolved in 2.1 ?

southerncomputer commented 8 years ago

Nope! Still 3.5meg bundled javascript!

TommyKolkman commented 8 years ago

With all respect, but this is not an improvement or feature, but a bug, if you ask me. Right?

leoquijano commented 8 years ago

I agree with @TommyKolkman: this function is unusable as it is right now, so it looks more as a bug and not as a feature.

pboisvert commented 8 years ago

this is being tracked internally as a bug and should not have the improvement label

erikhansen commented 8 years ago

@pboisvert Glad to hear it. I would love to be able to use the JS bundling feature, but unless it's refactored to be smarter (as I suggested in a previous comment), it's basically unusable.

While the coming adoption of HTTP/2 will make it more viable to run M2 without bundling (due to multiplexing), I expect that if smart bundling is implemented in conjunction with HTTP/2, it will result in better performance.

leoquijano commented 8 years ago

I think this issue report should be reopened, right?

Khaleel commented 8 years ago

The entire front end using RequireJS has so much bloat and overhead - can this be sorted?

pboisvert commented 8 years ago

reopening just to confirm this is being tracked as a bug. We have an issue for this as noted above that is open.

Khaleel commented 8 years ago

Thanks @pboisvert

Zaylril commented 8 years ago

@erikhansen Just as a side note we have a few sites on CloudFlare which essentially serves all the JS files via HTTP2. It makes little difference to the performance of the checkout (250+ JS files) and other areas of the site. Performance improvement imo needs to be made to the component/widget and template rendering, especially the checkout, which in it's current state is pretty awful speed wise.

hostep commented 8 years ago

@Zaylril: that's exactly what we are seeing unfortunately. HTTP/2 makes little difference because the js files are being requested by other js files which in their turn also request other js files, and so on and so on (if I remember correctly, it has been a few months ago since I last looked at this). So the js files aren't all being requested simultaneously, which makes using HTTP/2 not really helpful in this case. If all the js files would be referenced from within the html, we would see a bigger jump performance wise I think (when using HTTP/2). So for the time being, we have enabled js bundling and that seems to be the best we can do for now.

Khaleel commented 8 years ago

If all the js files would be referenced from within the html, we would see a bigger jump performance wise I think (when using HTTP/2).

This is not entirely true. Magento 1 still does this - they include all the prototype, language, and all JS files right at the top of the application and this casues it to be slow. It needs configuration out of the box and a CDN only helps half way.

It is so disappointing about the Front End Development architecture. Magento has outsourced all these European developers and you can see that they have lack of experience (when I say experience I am talking working with 10+years on the web within ecommerce companies) and that the other developers have not fully thought things thru.

The collaboration between coders is not working I think.

viktor-zhuromskyy commented 8 years ago

It has been fixed in 2.1.1 Now it produces close to 5MB, and when delivered to the client as gzipped content - the file size is 1MB! That's super nice!

erikhansen commented 8 years ago

@devdesco-ceo Thanks for sharing. I've confirmed that turning on bundling in 2.1.1 results in a ~4.3MB file (~900KB compressed).

09-31-26 checkout-p9mdy

As a point of reference, if you visit a 2.1.1 homepage with bundling turned off, the page will return 120+ JS files that are 1.9MB uncompressed. So while bundling will double the overall file size, it is only a single file.

I'd be curious to see tests comparing the difference between bundling enabled vs disabled. Using Chrome's Network Throttling feature would allow for comparing the performance difference at network speeds.

viktor-zhuromskyy commented 8 years ago

@erikhansen You are absolutely right about the sizes. In my case with bundling turned on website load size is roughly 2MB, but without bundlin being on, but merging and minification ON, the overall size is about 1.6MB. So that's 400 MB savings on every new visit. More than that, JS bundling is still broken, since I noticed it is breaking ./prototype/. and ./jquery/. paths in static view folder. So, I disabled it for good, especially with HTTP/2 enabled Nginx 160 files load vs 65 files to load is not an issue, hence these are piped via single socket, so to say...

viktor-zhuromskyy commented 8 years ago

To add to the discussion, with proper implementation of PHP OpCache, CDN, caching, minifications and merging, my page load is between 3.1 and 6 seconds that gives a green signal on GTMetrix, depending on a page complexity.

pynej commented 8 years ago

Still major preformance issues on /checkout/ the rest of the site is fine with Varnish, bundling, and combining fills all enabled.

For our site the /checkout/ still takes 3.5s to generate and download the Html and its 490kb, and the combined script file is 3.4MB and takes 6s to download. This is a hell of a lot better then disabling bundling(then we get >100 requests and the page still takes >6 seconds to load, though it only downloads 2.1MB instead of 4MB.) The savings in size is lost in multiple requests though so its not any faster.

mikel5 commented 7 years ago

We've discovered that bundling isn't usable because it requires localStorage. And localStorage isn't available when in Private browsing mode on iOS devices. And there are no try/catch checks around the localStorage calls.

Khaleel commented 7 years ago

@mikel5 Thank you for that insight - I was wondering why we get issues here and there - will look into this.

What I do temporary fix for now:

Bundling is a science and in 12 years of ecommerce I have never seen an app, be it Gulp, Grunt or even Magento bundle JS files together correctly. It is a complicated procedure and unfortunate it has not be cracked well. Even CloudFlare does not do it well. I never bundle JS files unless I do them manually.

pynej commented 7 years ago

Never? https://github.com/Shazwazza/ClientDependency is a .Net package that does this and more with ease. Minification, compilation, caching, versioning. It's really not rocket science.

Jeremy Pyne

"Fear is the mind killer."

On Nov 14, 2016, at 11:48 PM, Khaleel ibn Anwar Mughal notifications@github.com wrote:

@mikel5 Thank you for that insight - I was wondering why we get issues here and there - will look into this.

What I do temporary fix for now:

All files on a CDN (CSS, JS, Images) All files minified manually, including the major JS libraries in vendor Bundling is a science and in 12 years of ecommerce I have never seen an app, be it Gulp, Grunt or even Magento bundle JS files together correctly. It is a complicated procedure and unfortunate it has not be cracked well. Even CloudFlare does not do it well. I never bundle JS files unless I do them manually.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

Khaleel commented 7 years ago

Any updates on this? Knockout and the checkout is massive https://github.com/magento/magento2/issues/4868

zchking commented 7 years ago

Magento2.1.2 get 6M+ js file after merging, any update now?

zchking commented 7 years ago

does magento 2.1.3 solve this issue?

viktor-zhuromskyy commented 7 years ago

Did not check the 2.1.3 yet, but in 2.1.2 I had to manually inject requirejs-config.js into frontend and adminhtml in order to solve the issue.

zchking commented 7 years ago

thank you:)