RequestReduce icon indicating copy to clipboard operation
RequestReduce copied to clipboard

Support async and defer script attributes

Open mwrock opened this issue 13 years ago • 11 comments

Bundle all scripts marked async or defer in their own bundles with this attribute added.

mwrock avatar Jan 27 '12 13:01 mwrock

I found a nice little way of handling async/defer. It needs some RegEx and tests, but the skeleton is there, and the test is green.

https://github.com/madsstorm/RequestReduce/commit/71e1b6d9f1440ad716c5b6d5203ebde918b4ba60

madsstorm avatar Feb 09 '12 21:02 madsstorm

Hi Mads. Great stuff. Before you get too far along let me share some thoughts I have for this feature. Its a feature I'm really excited about. What I'd really like this to do is do more than merely provide support for scripts that use async and defer but to detect these attributes and perform best practice optimizations beyond what the simple inclusion of the attributes can accomplish.

By specifying either defer or async, a script now allows RequestReduce's bundling to behave differently. Perhaps most notably is that RequestReduce is no longer bound to preserve the location of the script. RequestReduce could now bundle all of these scripts together regardless of their location on the page resulting in fewer http round trips. This being the case it also make sense to bundle them all and place them at the very end of the page which perhaps is obvious. Furthermore, depending on combinations of the use of async and defer, RequestReduce could go even further and perform dynamic script injection to speed script execution and support async for IE <= 9.

The trick here is determining what behavior is appropriate for the various combinations of defer/async usage. An underlying principle of RequestReduce is that it should never "break" the page or cause the markup to behave in a way that deviates from the intent of the markup used unless it is granted special permission via a api or config option (or perhaps a custom data attribute). So while the usage of these attributes allow RR to possibly optimize the page beyond simply preserving the attributes it should not assume that it can perform "best practice" script loading if there is a chance that unexpected behavior will result. Of course the fact that RR essentially ignores these right now already violates this principle.

So ideally here is how I see usage of these attributes handled:

  • Async only AND Async used with Defer - Both scripts that use async and scripts that contain both async and defer can be handled using the same behavior because the behavior of scripts including both attributes is to fall back to defer ONLY if the browser does not support async. So unless you are in IE version 9 or below, the async behavior will always be used. Now lets say you are in IE <= 9. Even though the async attribute is not supported, you can force async behavior by using script injection. So ideally I think requestreduce would bundle all scripts that have async or derar with async and at the end of the body, dynamically inject them to the start of the dom with async set to true.
  • Defer only - By using defer only, the author is stating that the scripts should not block rendering but that they should execute in order. Now there has been plenty of observations that while this is the intent of defer, several browsers do not honor this as one would expect but because RequestReduce can bundle scripts in the order they appear on a page it can effectively gurantee that this intent is honored. These scripts should be bundled together and rendered in a normal script tag at the end of the body.
  • Scripts already at the end of the body that don't have either attribute - These should have the same treatment as scripts with defer only. They should be bundled together with the defer only scripts all in the same order they appear in the markup and loaded at the end of the body together.

Now this all said, I think that your current implementation is definitely a step in the right direction and undoubtedly better than RequestReduce's current implementation. Perhaps in the interest of getting some low hanging value out quickly, you should keep to the current logic. I just wonder if this logic is really all that much more work. I'll let you decide that, but here are some implementation options:

  • Create instance variables in ResponseTransformer as a holding buckets(ILists) for both Defer and Async along with axync/defer scripts. A ResponseTransformer instance is scoped to a single httpRequest. So multiple requests to ResponseTransformer.Transform in the same Http Request will occur in the same instance.
  • as you encounter these, add them to the bucket and dont bundle them in the current call.
  • Somehow ResponseTransformer.Process has to know when it is being called for the last time in a request (more on this in a bit) which currently it doesn't. On the last call, bundle the scripts in the buckets and render them according to the methods stated above.

How to know when ResponseTransformer.Process is called the last time - I think you'd have to modify Response.Filter to look for the closing </body>. This is probably the trickiest part of all of this. Upon seeing the closing body tag, the filter could either pass a boolean parameter to ResponseTransformer.Process indicating that it is the last call or it could include the closing body tag in the preTransform string and the ResponseTransformer, seeing that, would know to bundle and render the js.

Boy that was a long comment. Sorry about that, but this is something that could really be a "killer" feature. No other OSS bundler does this. It would be a great way to enforce best practices in an unobtrusive way when best practices are either forgotten or intentionally left to RequestReduce.

mwrock avatar Feb 10 '12 14:02 mwrock

Hi Matt,

That's some really cool visions you have for RequestReduce and definitely goes beyond my little bundle "hack". Supporting async and defer for browser that dont do natively, would be cool. The current "one-resource-at-a-time" bundling would need some additional state for "scripts-to-handle-later", and where to insert those scripts "later". Sounds like a fun challenge :)

If we simulate "async" in IE by inserting some dynamic-load-scipt, but render that load-script at the end of the body, would we loose the benefit of "async"? Both defer'ed and async would then be loaded after the document is parsed. "Defer" by static script-tags and "async" by a dynamic-load-snippet, but still after parsing the whole page. I guess "bundling" and "async" are kind of opposites.

I'll try and see how far I can take my bundle-code.

madsstorm avatar Feb 10 '12 22:02 madsstorm

Perhaps the async-load-snippet could be placed at the position of the last async script (not at < / b o d y>), to start the parallel download as soon as possible. Im assuming here that including an async script earlier than its original location could potentially break something.

Maybe a config-setting for bundling/not-bundling async scripts, depending on the nature of the site, amount of script etc.

madsstorm avatar Feb 11 '12 16:02 madsstorm

Excelent point on the draw back of injecting async tags at the end to simulate async for IE.

CONS:

  • non ie browsers(~50%) have to wait for the markup to be parsed for async scripts to start loading

PROS

  • non ie browsers load all async scripts via a single http request
  • IE gets async

The question for non IE browsers is does the benefit of a single async HTTP request at the end of page parsing outperform multiple async httprequests earlier on. Of course the answer is a big "It Depends" here but RequestReduce can't really know enough to weigh the factors that would determine what's best.

The ideal solution may be to detect the browser here and use different logic for IE <=9 and other browsers. IE<=9 gets the logic I propose and other browsers handle async exactly as you handle it now.

mwrock avatar Feb 11 '12 16:02 mwrock

Just saw your last comment. I think the problem there is because RR filter is parses in a streaming fashion, there is no way to know that an async script is the last one until you actually get to the </body> tag.

mwrock avatar Feb 11 '12 16:02 mwrock

This is going pretty well. The bundled async and defer scripts are inserted just before body-close now. Its up and running on my own site.

Im collecting the scripts and emptying them into the stream on 3 events:

  • body-end
  • html-end
  • stream.Close , so the accumulated scripts will eventually get inserted, even with weird markup.

Since the ResponseTransformer never sees the body-tag or html-tag, this is controlled from the ResponseFilter, that will insert the deferred scripts, just before writing to the Stream. It also knows when its about to close itself, and can write any leftover scripts.

The async scripts are inserted with a dynamic load-snippet with a lot of inspiration from Facebook/Google's snippets, so it should work with the strangest markup.

So whats left ? It needs some solid RegEx for recognizing async/defer-scripts. I dont think my RegEx-skills are good enough for that. Async-scripts are always bundled and inserted at the end now. Im not sure if we want some conditions here, like "IE" or some config. And finally it needs a lot of tests !

madsstorm avatar Feb 12 '12 15:02 madsstorm

Sounds Great! Maybe I'll pull down your fork in the next couple days and play with some of the regex. In my 13 years of professional development life, I managed to get away with 12 without doing hardly any regex and what little regex I did do was trivial. Thanks to RequestReduce, those happy times are over :)

There are two regex resources that I now use probably several times a week:

  • http://regexhero.net/ - A great in browser .net real time regex parser. Its actually free. You just have to ignore the popups but I really should just buy it.
  • http://www.regular-expressions.info/reference.html - has become my regex cheat sheet. Its a good reference for all regex syntax.

mwrock avatar Feb 13 '12 15:02 mwrock

I found a very handy "TagRegex" inside ASP.NET that does all the attribute parsing for me. So I guess I'm done :-)

madsstorm avatar Feb 13 '12 21:02 madsstorm

I ended up having more coupling between the filter and the transformer, than I really like. The filter shouldnt really know anything about what the transformer is doing. To get rid of that would require modifying the filter state machine to send standalone body-end-tag and html-end-tag, and also somehow signal stream-close. At least my nasty tests should make a refactoring safe.

madsstorm avatar Feb 14 '12 07:02 madsstorm

Good deal. Looking forward to looking this over. Just been super duper busy lately and most RR efforts have been bug fixes or features specifically for work scenarios. Once this gets all cleaned up and pulled in, I think I'll cut this as v1.8 and blog on this of course giving you due credit!

mwrock avatar Feb 14 '12 14:02 mwrock