Skip to content
This repository has been archived by the owner on Dec 1, 2023. It is now read-only.

[Feature Request] Enqueue scripts in footer. #550

Closed
AlexanderOMara opened this issue Feb 28, 2016 · 7 comments
Closed

[Feature Request] Enqueue scripts in footer. #550

AlexanderOMara opened this issue Feb 28, 2016 · 7 comments

Comments

@AlexanderOMara
Copy link

Re-creating old issue from pre-beta as it seems to still be applicable.

Right now it appears that all scripts in theme and extensions are enqueued in the head. Generally, it's better to include scripts in the footer when possible for performance reasons.

It would be helpful if @script had an option to print the script in the footer, rather than in the header, perhaps by having these print in a @section('footer') section in themes.

@malte-christian
Copy link
Contributor

I added support for the script tag attributes 'defer' and 'async' (2081cc1).
The option can be set on registering the script with:

 $app['scripts']->register('some-script', 'script.js', [], ['async' => true]);
 $app['scripts']->register('some-script', 'script.js', [], ['defer' => true]);

I think this should bring apart from IE < 10 the same performance gains as enqueueing scripts in the footer. What do you think?

@floriandotpy
Copy link
Contributor

Some background on why we prefer this over rendering in the footer: http://www.growingwiththeweb.com/2014/02/async-vs-defer-attributes.html

Browser support looks good: http://caniuse.com/#feat=script-defer

@AlexanderOMara
Copy link
Author

That's pretty good too, probably good enough for my purposes.

The IE <= 9 bug is pretty nasty though, especially if jQuery is affected by it (maybe jQuery has been adjusted since?).

On this new feature, is there any way to make the PageKit provided scripts like jQuery use the defer attribute?

@malte-christian
Copy link
Contributor

Since the IE <= 9 bug only occurs if scripts depend on each other, I think it is manageable. If you need IE 9 support and depending script with the defer attribute, you can employ for example some kind of browser detection and remove the attribute for IE <= 9.

An other option is to load scripts async with Pagekits Asset Mananger.

@AlexanderOMara
Copy link
Author

Was there a way to get existing scripts to load in a non-blocking fashion? For example, the default theme has 10 blocking script tags in the header.

Also, for future visitors, outputting different HTML based on user-agent can cause problems with some reverse-proxies or CDN's, if they do not also serve different HTML for that user-agent.

This was referenced Apr 21, 2016
@jdegger
Copy link

jdegger commented Jun 22, 2016

Just a heads up the current documentation for the defer and async options are incorrect. At least for php5 (haven't tested with 7).

https://pagekit.com/docs/developer/views-templating#working-with-assets

<?php $view->script('theme', 'theme:js/theme.js', [], ['defer']) ?>

Does not work even though:

<?php $view->script('theme', 'theme:js/theme.js', [], ['defer' => true]) ?>

works fine.

created a pull request for this in the docs

jdegger added a commit to jdegger/docs that referenced this issue Jun 22, 2016
@jdegger
Copy link

jdegger commented Jun 22, 2016

The defer option does not affect the dependencies:

<?php $view->script('theme', 'theme:js/theme.js', ['uikit-sticky',  'uikit-lightbox',  'uikit-parallax', 'uikit-datepicker'], ['defer' => true]) ?>

Renders:

<script src="/app/assets/uikit/js/uikit.min.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="/app/assets/uikit/js/components/datepicker.min.js?v=cae3"></script>
<script src="/packages/pagekit/theme-avion/js/theme.js?v=cae3" defer></script>

Note that only the actual theme.js file has the defer attribute. I'm not arguing all the dependencies should automagically also get the defer attribute but there is no way to accomplish this now without creating a ->script() call for every dependency (and defeating the purpose of the dependency argument in the first place).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants