Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for JS aggregation/minifying #682

Open
d9media opened this issue Jun 19, 2016 · 15 comments

Comments

@d9media
Copy link

commented Jun 19, 2016

More than 30 js files are being loaded (using Theme One that is). There should be an option to aggregate and/or minify those assets for performance reasons.

@jdegger

This comment has been minimized.

Copy link

commented Jun 22, 2016

+1, there is a lot of resources now and they can easily be concatenated

@d9media

This comment has been minimized.

Copy link
Author

commented Jun 22, 2016

By the way - I've solved it in another way. Have look into Google's modPageSpeed library for Apache/nginx. Once you have your setup, the results are really impressive. Although some settings do not play well with the backend, but that can easily be solved by disabling the module on any admin/* location.

But in general it would be awesome if Pagekit had a setting for simply aggregating all the package's JS files.

@jdegger

This comment has been minimized.

Copy link

commented Jun 22, 2016

modPageSpeed is amazing and I have all the proper optimizations installed on my server but an unminified source code + non concatenated resources are still not the best practice.

Appreciate your input though :)!

@jdegger

This comment has been minimized.

Copy link

commented Jun 23, 2016

Just to have another example of the extend of this problem:

<script src="/app/assets/vue/dist/vue.min.js?v=cae3"></script>
<script src="/app/assets/jquery/dist/jquery.min.js?v=cae3"></script>
<script src="/app/assets/lodash/lodash.min.js?v=cae3"></script>
<script src="/system/intl/nl_NL?v=cae3"></script>
<script src="/app/system/app/bundle/vue.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/bixie-framework.js?v=cae3"></script>
<script src="/app/assets/uikit/js/uikit.min.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/tooltip.min.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/bixie-fieldtypes.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-email.js?v=cae3"></script>
<script src="/app/system/modules/site/app/bundle/panel-link.js?v=cae3"></script>
<script src="/app/system/modules/finder/app/bundle/link-storage.js?v=cae3"></script>
<script src="/app/system/modules/site/app/bundle/link-page.js?v=cae3"></script>
<script src="/app/system/modules/user/app/bundle/link-user.js?v=cae3"></script>
<script src="/packages/bixie/formmaker/app/bundle/link-formmaker.js?v=cae3"></script>
<script src="/app/system/modules/site/app/bundle/input-link.js?v=cae3"></script>
<script src="/app/system/modules/editor/app/bundle/editor.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-htmlcode.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/form-select.min.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/datepicker.min.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-dob.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-textbox.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-pulldown.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-checkbox.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/upload.min.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-upload.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-text.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-radio.js?v=cae3"></script>
<script src="/packages/bixie/framework/app/bundle/fieldtype-sitelink.js?v=cae3"></script>
<script src="/packages/bixie/formmaker/app/bundle/formmaker.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/sticky.min.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/lightbox.min.js?v=cae3"></script>
<script src="/app/assets/uikit/js/components/parallax.min.js?v=cae3"></script>
<script src="/packages/pagekit/theme-avion/js/theme.js?v=cae3" defer></script>

This is just a ridiculous amount of requests to display a form.

@jdegger

This comment has been minimized.

Copy link

commented Jul 20, 2016

Note: enabling CloudFlare Rocket Loader is a relatively simple workaround for now. By no means a "real" solution though.

@jdegger

This comment has been minimized.

Copy link

commented Jul 22, 2016

In addition, there seems to be code combine assets but it is not being used? @MalteScharenberg

@MalteScharenberg

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

In the current Pagekit version we reduced the scripts which are loaded by default and we are also experimenting with the 'defer' attribute to mitigate the effects of multiple script files on https://pagekit.com. Modern browsers will download deferred scripts asynchronous, in parallel over multiple TCP connections (or are even capable of reusing a TCP connection for further files), so I think that the difference to one big file is negligible.

Problem with aggregating all script into one file is that you circumvent effective caching if the required scripts differ from page to page and it is hard to determine sets of scripts which make sense to combine automatically.

I think combining scripts in a meaningful manner should be the job of the extension (in your example Bixie Formmaker).

@jdegger

This comment has been minimized.

Copy link

commented Jul 25, 2016

I agree with everything you said. Just a concat_all() would definitely not do the trick. I'll make an issue at bixie/pagekit-formmaker to have issue with formmaker addressed.

That being said, I noticed that adding defer to a script request with dependencies only adds it to the main script, not to all the dependencies. Thanks for your response.

@MalteScharenberg

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

The idea is that it should be possible to require a script with all its dependencies deferred. Currently this is not implemented in the public release, because this could break some JS (you can only see it at https://pagekit.com where we are experimenting with it).

@jdegger

This comment has been minimized.

Copy link

commented Jul 25, 2016

What happens when you have the following scenario:

Script 1 (defered / async)

  • jQuery
  • Lightbox

Script 2 (normal sync load)

  • jQuery
  • Something else

Assuming the assets manager doesn't include jQuery twice if it's called as a dependency twice. Is jQuery now loaded defered or not?

@MalteScharenberg

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

In this case jQuery would not be deferred because it would otherwise break script 2.

@jdegger

This comment has been minimized.

Copy link

commented Jul 25, 2016

great :)

@FDiskas

This comment has been minimized.

Copy link

commented Nov 11, 2016

🎸 Actually HTTP 2 is coming. My hosting provider is supporting this already. So minifying or concatenating makes no sense anymore.

@yaelduckwen

This comment has been minimized.

Copy link

commented Dec 26, 2017

+1

@humantex

This comment has been minimized.

Copy link

commented May 19, 2019

Sorry to say, but... HTTP/2 being enabled (and tested) on my sites does nothing in the way of bettering the corresponding parts of the scores at PageSpeed Insights, or GTmetrix, etc., when it comes to resources used. Anything not properly set as async or defer, will NOT magically disappear from the list of render-blocking resources just because HTTP/2 is in use.

A fresh install of PageKit 1.0.16 running under PHP 7.2.8, on a server using the latest version of Lightspeed, results in the following:

  • GTMetrix (YSlow tab) reports an "F" score (10% out of 100%) in 'Use cookie-free domains' (Render-Blocking Resources), with 16 scripts reported in the list. Trying to use defer or async in theme code does nothing to change that - as it's already been noted.
  • PageSpeed Insights gives a site score of 76% out of 100% for mobile devices, with the majority of the non-content issues centered around those same resources, when not being loaded as async or deferred.

I have absolutely no issue with my current page speeds from PageKit sites running on my server setup (Home page: 0.9s, slowest page, 3.6s, average page, 1.9s) but the bigger concern for me, is the render-blocking issues (i.e., "11 of 27 resources not loaded") that I've been chasing for months that can cause indexing errors with Google search. Crawling times are exceeding whatever arbitrary limits they've set, and the crawler can't load all the resources it's expecting and will invalidate a previously indexed page, or will fail to validate a newly added page to be included in its index. There's much more at play here than simply getting faster site/page load times.

There is an online test to verify if any site is using HTTP/2 - available here: https://tools.keycdn.com/http2-test

My latest site tests:
GTmetrix: https://gtmetrix.com/reports/for-sale.lumiworx.com/1xCeQ0vr
PageSpeed (74% Mobile, 90% Desktop): https://developers.google.com/speed/pagespeed/insights/?url=https%3A%2F%2Ffor-sale.lumiworx.com%2F

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.