huge longshot, but... might be a bug with LAB.js here.
kylebakerio opened this issue · 8 comments
Having a really weird issue, LAB.js is critical to it, and likely near the root cause, could potentially be a bug with it...
Short version (trust me, I've been poring over this all day):
I've got a big list of scripts, including angular and non-angular stuff (1.2.x). When I try to use LAB.js to load it all, including AlwaysPreserveOrder flag, Angular fires too early. Digging in, it seems Angular is using $(document).ready(/* start angular */). Understandable, script loaders undermine the normal document.ready event.
So, I try to use $.holdReady(true) and $.holdReady(false), but, I get really strange behavior: While $.holdReady(true) does increment $.readyWait as expected, it just won't prevent angular from initializing... and on top of that, if I try to set my own listener for $(document).ready(), it never fires, even though angular is seemingly using that same event to fire...
...To add to the mystery, every once in a blue moon, if I set it to keep doing window.location.reload() on fail until things work, it actually will just work--very rarely. That makes it seem like a race condition, but given that I'm passing in the order flag, I can't imagine what would be racing, nor does that clue seem to fit with everything else, as far as I can tell...
Note that if I switch to non-lab.js version of the file, everything seems to work just fine, including
I absolutely don't expect you to dig into it in depth, but if you wanted the long version with code examples, I've detailed it here.
If you had a clue, though, I'd love to hear it. I'm really at my wit's end here.
Based on your description, this doesn't sound like a bug in LABjs. First off, we'd need to know which browser are you using for this test? If it's a modern one, I can virtually guarantee it's not a LABjs bug, because under the covers, LABjs delegates to the browser to enforce loading order (using the async = false "ordered async" feature).
Understandable, script loaders undermine the normal document.ready event.
No, I wouldn't characterize this as undermining anything. Expose false assumptions? Yes.
Historically there have always been race conditions, and always will be, when people (ie, developers of projects) conflate "DOM ready" with "scripts have loaded". I ranted about this countless times ages ago. Here's one relevant issue thread I found in my searching.
It sounds like Angular is assuming that DOM ready means "all dependency scripts have loaded". That's not at all what DOM ready means, and even less so when there's dynamic script loading involved.
So, I try to use
$.holdReady(true) and $ .holdReady(false)
I don't know what these are, as I'm not an Angular dev. I can't really comment on whether they're safe for dynamic script loading or not.
That makes it seem like a race condition, but given that I'm passing in the order flag
It probably is a race condition, but it sounds like it's not a race condition between scripts but rather a race condition between scripts finishing loading and the DOM ready event being assumed to mean the scripts finished loading.
My advice continues to be: stop relying on DOM ready to mean "scripts are loaded". Any framework that does that is being openly dismissive of reality.
I've run into your rants while researching this issue, actually, haha. You could say it was poor word choice to say "undermine", but even then I think I'd defend the word--it undermines their (faulty) assumptions. I'm not saying it's LABjs' responsibility to respect those assumptions, mind you.
That being said,
- These false assumptions are those of the angular code here, not my own code. I'm trying to accomodate Angular here. LAB.js just seems to not play nicely with Angular. (I am no angular fan, this is legacy code I'm trying to patch up well enough to get a rewrite underway.)
- It's a jquery event (it's not the same as window.onload, and is not a real DOM event as far as I am aware; it has its own characteristics in its attempts to be cross-browser and support its assumptions), and as its name tells us, its intention was to signal that code running then can expect everything to have loaded, so yours can run when things are "ready".
I agree that it's poorly conceived, because it bakes those assumptions straight in.
$.holdReady() is a jquery tool to manipulate the
I totally agree that Angular is broken here, and has a shockingly poor assumption built in, but I'm really baffled at how LAB.js seems to somehow be interfering with jquery--not just by undermining its assumption, but for the fact that for some reason, when I use LABjs, the
Regardless, thanks for your time. Always appreciate your input.
These false assumptions are those of the angular code here, not my own code.
I didn't mean to suggest it was your fault or poor assumptions. The fault is with tools that make these decisions, always has been and always will be. :)
Moreover, these bad assumptions make those tools unfriendly to any dynamic resource loader, even a manual inline snippet inside a <script> tag. Dynamic resource loading, which has been a thing for a really long time, is antithetical to these assumptions. It's similar to tools that rely on document.write(..). I've literally had such tools just tell their users "don't dynamically load".
It's a jquery event (it's not the same as window.onload, and is not a real DOM event as far as I am aware; it has its own characteristics in its attempts to be cross-browser and support its assumptions)
Are you talking about $.ready(..) here? I'm assuming so. "DOM ready" is otherwise known as "DOMContentLoaded" and is absolutely a real DOM event thing. All $.ready(..) does is provide a dedicated event handler interface for it.
And BTW, jquery's $.ready(..) is patched (as of jQuery 1.4 years ago) to handle the use case where jQuery itself is dynamically loaded after DOMContentLoaded has already passed. What jQuery does (or at least used to do, I haven't looked at their source code in half a decade) is check the document.readyState property (again, totally standard DOM stuff) to check to see if the document load has already occurred. If so, jQuery always fires your $.ready(..) right away.
But all of this is actually orthogonal to your problem. The problem is not whether $.ready(..) works or not. In fact, $.holdReady(..) is not fixing anything "broken" about $.ready(..). It's making $.ready(..) work in an entirely different way than it was designed, because the authors of these frameworks/tools insist on using it differently. But DOM-ready has worked -- as designed -- for a really, really long time.
In fact, the proof that it "works" is that it's firing for you, even though it's firing "too early". The separate problem is that this event will, in general, always be able to fire before resources that don't block the event -- any dynamically loaded resource, any resource that defers its loading (like <script defer>, etc), and more -- has finished loading.
If a resource doesn't block the event, obviously there's a race condition.
as its name tells us, its intention was to signal that code running then can expect everything to have loaded
Eh, no. Though the name DOMContentLoaded says "Content", the "content" here isn't about external resources. It's about the internal DOM element structure. And "Loaded" means the HTML itself has loaded and been parsed into those DOM element objects.
Quoting the MDN page:
The DOMContentLoaded event is fired when the initial HTML document has been completely loaded and parsed, without waiting for stylesheets, images, and subframes to finish loading. A very different event
loadshould be used only to detect a fully-loaded page.
So document.ready is definitely not about "resources have finished loading." window.onload was originally meant for resource loading completion, even though it also won't wait for (some kinds of) dynamically loaded resources.
The crux of the mal-assumption around DOMContentLoaded has always been related to <script> elements. Because document.write(..) needs to run synchronously, the browser has always ensured that any <script src=..> or <script>..</script> element would execute (regardless of loading order) in the same order as listed in the DOM.
Thus, the logical reasoning goes: "if the current line of code is executing, then any other code that was 'requested' (listed in the DOM) earlier must already have finished loading AND EXECUTING." From that, it's a natural extension to assume, "if i declare a DOM-ready handler in a piece of code, then all the code -- at least the code 'before' me -- must already have loaded and run, because I couldn't have gotten to this line of code if that previous code hadn't already run."
Long before resource loaders came around, this assumption was already broken when HTML added <script async> and <script defer>. In both those cases, you're telling the browser to take that script resource and treat it as a separate thing that doesn't hold to those rules. So it's really always been a terrible assumption and conflation, but resource loaders painfully expose it for what it is: junk science.
$.holdReady() is a jquery tool to manipulate the $ (document).ready event
Your bug may just be that this plugin is broken or incomplete. I don't know.
But in theory, if it lets you "hold off and artificially fire jquery's ready event whenever" you want, then you should be able to "delay" until the final .wait(..) in your $LAB chain. That may be what you were trying to do, not sure, but at least in theory it seems reasonable. Well... hacky patchy ill-advised... but not irrational.
Now that I think about it... if $.holdReady is a jQuery plugin -- it modifies jQuery at run time -- the race condition could just be that jQuery is firing its ready event before $.holdReady has a chance to modify jQuery to hold the event off. It could be that you need to package jQuery and $.holdReady in the same file so they're ensured to run together. I dunno.
I still really feel like this is a bug that has nothing to do with LABjs, but more about the process of resource loading in general, and perhaps a failure to hack things to work differently than designed.
IOW, I'm pretty sure I could expose this bug without using LABjs at all, just using basic resource loading logic.
Any chance you could pull out the part of angular that is creating the problem (I guess along with $.holdReady(..)) and show a standalone minimal reproduce test case that I could analyze, instead of needing to grok all of angular's internals?
jQuery's ready is indeed a proxy for DOMContentLoaded, except for old IE. I thought it had some other checks in there, but I suppose it's just those ie checks and the late-event-listener support.
Since scripts can modify the DOM, you can't know if the DOM is in a final state until all ajax requests have returned and the results have executed. This is your point, of course--there is no guarantee on DOM state, you have to handle that yourself.
So I hear what you're saying, but I guess I was trying to say that I was under the impression that DOMContentLoaded was around before ajax in javascript was. I can't tell for sure, and it seems like that might not be true (iframe technically enabled a crude form of ajax back in '98, even if ajax as we know it today wasn't around until gmail came out, really), but it was around before the async script attribute (2010?) was for sure. (can't tell exactly when DOMContentLoaded, but it was in v 1.0 of ff... but Wikipedia says it was unofficial before HTML5, which is interesting.) Mypoint is that my guess had been that the spec writers didn't think about the possibility of async JS code modifying the DOM, or that that wasn't possible yet, but I now think I'm probably mistaken in that belief.
This is what I meant by saying "ready" was "broken", but if you instead postulate it was correctly defined all along, then yes, it's just its widespread use that is broken.
...though at that point one has to wonder whether something that is so widely misunderstood has an inherent flaw, no? I adhere to your principles of 'code is written for humans first', shouldn't this really be called DOMResourcesExecuted or something that makes it a bit more clear, since one can never know if the DOM is actually "Loaded" (semi-ambiguous word that carries a sense of synchronous finality within an async environment)?
Anyways, I'm fascinated by discussions like this, but I don't mean to waste your time. It's been an enlightening digression for me.
I've literally had such tools just tell their users "don't dynamically load".
Yeah, that seems to basically be the case with Angular here. All they would have to do is have their stuff execute on a given event, and tell users to fire that event when ready.
But in theory, if it lets you "hold off and artificially fire jquery's ready event whenever" you want, then you should be able to "delay" until the final .wait(..) in your $LAB chain. That may be what you were trying to do, not sure, but at least in theory it seems reasonable. Well... hacky patchy ill-advised... but not irrational.
That's exactly what I was doing, indeed. Agreed somewhat hacky, intercepting the jquery.ready event, which is why I referred to it as a "patch attempt"... though, to be fair, jquery supports it itself, and jquery.ready is their own API. Though, to be fairer still, shit doesn't work right now, probably because it's all hacky.
But in theory, if it lets you "hold off and artificially fire jquery's ready event whenever" you want, then you should be able to "delay" until the final .wait(..) in your $LAB chain. That may be what you were trying to do, not sure, but at least in theory it seems reasonable. Well... hacky patchy ill-advised... but not irrational.
I was able to produce this bug even though I was putting the script tag for jquery at the top of my head, and an onload="$.holdReady(true)" attr for it. I read in a couple places (but did not see it mentioned on MDN's documentation for <script>...) that script tags in the header are async by default, so I moved it to the body, but saw the same issue, unfortunately, no difference <-- I was accidentally loading jquery twice from moving things around during that experiment, whoops. That does seem to have potentially made a difference, and that seems like pretty poor design upon that discovery; I wouldn't expect async=true default in one place and async=false in another. Alas, I'm sure that was likely a part of things, I'll experiment more and report in the next comment rather than pollute this with edits.
Here's the angular code bits:
if (window.angular.bootstrap) {
//AngularJS is already loaded, so we can return here...
console.log('WARNING: Tried to load angular more than once.');
return;
}
//try to bind to jquery now so that one can write angular.element().read()
//but we will rebind on bootstrap again.
bindJQuery();
publishExternalAPI(angular);
jqLite(document).ready(function() {
angularInit(document, bootstrap);
// this eventually fails because it expects things like angular.module("appname").controller('controllerName') to have been called by this point.
});
function bindJQuery() {
// bind to jQuery if present;
jQuery = window.jQuery;
// Use jQuery if it exists with proper functionality, otherwise default to us.
// Angular 1.2+ requires jQuery 1.7.1+ for on()/off() support.
if (jQuery && jQuery.fn.on) {
jqLite = jQuery;
extend(jQuery.fn, {
scope: JQLitePrototype.scope,
isolateScope: JQLitePrototype.isolateScope,
controller: JQLitePrototype.controller,
injector: JQLitePrototype.injector,
inheritedData: JQLitePrototype.inheritedData
});
// Method signature:
// jqLitePatchJQueryRemove(name, dispatchThis, filterElems, getterIfNoArguments)
jqLitePatchJQueryRemove('remove', true, true, false);
jqLitePatchJQueryRemove('empty', false, false, false);
jqLitePatchJQueryRemove('html', false, false, true);
} else {
jqLite = JQLite;
}
angular.element = jqLite;
}
The bind seems to work, and angular seems to use it, because in the console angular.element === $ yields true, and $().jquery yields 2.2.4 (the version I specify in my script, which wouldn't correspond to jqLite).
By the way, perusing the source of LABjs, I ran into this at the bottom... any thoughts on this coming into play, potentially?
/* The following "hack" was suggested by Andrea Giammarchi and adapted from: http://webreflection.blogspot.com/2009/11/195-chars-to-help-lazy-loading.
NOTE: this hack only operates in FF and then only in versions where document.readyState is not present (FF < 3.6?).
The hack essentially "patches" the **page** that LABjs is loaded onto so that it has a proper conforming document.readyState, so that if a script which does
proper and safe dom-ready detection is loaded onto a page, after dom-ready has passed, it will still be able to detect this state, by inspecting the now hacked
document.readyState property. The loaded script in question can then immediately trigger any queued code executions that were waiting for the DOM to be ready.
For instance, jQuery 1.4+ has been patched to take advantage of document.readyState, which is enabled by this hack. But 1.3.2 and before are **not** safe or
fixed by this hack, and should therefore **not** be lazy-loaded by script loader tools such as LABjs.
*/
(function(addEvent,domLoaded,handler){
if (document.readyState == null && document[addEvent]){
document.readyState = "loading";
document[addEvent](domLoaded,handler = function(){
document.removeEventListener(domLoaded,handler,false);
document.readyState = "complete";
},false);
}
})("addEventListener","DOMContentLoaded");
Actually, based on all the research I've done before and during this discussion, and the hunch that that snipped is interferring in some way, I decided to try $.holdReady() right before angular gets loaded. That fixed it, everything seems to work... Now going to try putting it right before, and right after, the LABjs script.
You know what, suddenly I can't reproduce the issue. That's really frustrating, and leads me to believe that I had just included jquery twice somehow, though I feel quite certain that I wasn't doing that... yet that would make sense with the symptoms I was seeing. A little embarrassing. The
Cheers, and thanks for the conversation. Talking this through led to the solution one way or another. :)
Cheers! :)
Cheers! :)