SlexAxton / yepnope.js

A Script Loader For Your Conditional Builds
http://yepnopejs.com/
BSD 3-Clause "New" or "Revised" License
2.51k stars 323 forks source link

Scripts prefixed with 'ie6!' are being loaded in every browser (I've tested). #19

Closed mkoistinen closed 13 years ago

mkoistinen commented 13 years ago

Hi again. I'm using the following in my HTML:

<script>
  yepnope({
    load: [ 
      "/js/jquery-1.5+tinypubsub-0.6.min.js",
      "ie6!/js/dd-belated-png.min.js"
    ],
    complete : function() {
      // Delay loading until DOM READY signal
      jQuery(function(){ yepnope("/js/front.js"); });
    }
  });
</script>

And, in Chrome 10.0.648.11, Safari 5.0.3 and Firefox 3.6.15 (Mac), I'm seeing dd-belated-png.min.js IS loaded. Am I doing it wrong? I have double-checked, I do not load that script in any other way. And the fact that it is loaded is evident when I print "window.DD_belatedPNG" on the console, it returns the relevant object.

The good news is, it IS loading in IE 6 too. =)

mkoistinen commented 13 years ago

I can confirm it also loads in IE8 and in IE8 in compatibility mode (possibly indicating it loads in IE7 too). And, sadly, DD-belated-png causes JS errors in IE 8 (strangely though, not in IE8 in compatibility mode.)

mkoistinen commented 13 years ago

OK, couple of things:

I would like to make these recommendations:

SlexAxton commented 13 years ago

Hmm, Okay, my feedback:

I understand that we seem to simply have a difference of opinion on most of these things, rather than any technical issues. Please know that with sufficient support, I'd consider these types of suggestions, but as of now, I think I like the structure as-is. I will certainly go back through and make the appropriate documentation changes to make these things more obvious in the future.

Thanks for your report! Alex

PS: you're example of what you're trying to do seems to go against the main point of yepnope, which is to load scripts in parallel. So rather than what you have there, perhaps you could not _automatically_runfront.js and instead have it create an object that you can call .init() on (or something similar). That way, you could do:

<script>
  yepnope({
    load: [ 
      "/js/jquery-1.5+tinypubsub-0.6.min.js",
      "ie6!/js/dd-belated-png.min.js",
      "/js/front.js"
    ],
    complete : function() {
      // Delay loading until DOM READY signal
      jQuery(function(){ Front.init(); });
    }
  });
</script>

And you'd get full parallel loading.

mkoistinen commented 13 years ago

Well, darn. I was hoping to use Modernizr.load/Yepnope to lazyload virtually all my JS until after Document Ready, and yes, I would like this to be in parallel where possible. (In this case, I don't need really need any part of front.js until after Document Ready, so, I think I'm doing this the right way for my case.)

Re: "You're example of what you're trying to do seems to go against the main point of yepnope, which is to load scripts in parallel". That's weird, I thought Yepnope's goal was to serve as a "Conditional Loader for your Polyfills"? =) And, what is DD_belatedPNG if not one of the original polyfills? =)

Re: "Modernizr.load is not guaranteed to act exactly the same as yepnope in the future". This is precisely why I would prefer to use Modernizr.load vs. a separate Yepnope build directly and therefore, precisely why I would like support for IE prefixes built in, or as an optional part of the custom Modernizr builder. Sadly, right now, we seem to be sitting across two stools.

I know that there's a stigma, nowadays about browser sniffing in favor of feature detection. I get that, 100%. And, I wish there was a feature detect algorithm for 32-bit PNG support, but there's not (that I am aware of). Until there is, we (web developers at large) need to know if we're on IE6. To be honest, I think this should be in Modernizr more-so than in Yepnope.

I can continue to use conditional scripts to load PNG support for IE6, but doing that ensures that, at least for IE6, DD_belatedPNG is a blocking load. A hybrid approach, albeit ugly is:

<head>
  ...
  <script src="/js/modernizr.min.js">
  <!--[if IE 6]>
  <script>
    var ie6 = true;
  </script>
  <![endif]-->
<body>
  ...
  <script>
    yepnope({
      test: window.ie6,
      yep: "/js/dd-belated-png.min.js",
      load: [ 
        "/js/jquery-1.5+tinypubsub-0.6.min.js"
      ],
      complete : function() {
        $(function(){ yepnope("/js/front.min.js"); });
      }
    });
  </script>
</body>

You can imagine that this would get even uglier if, for some reason, I needed to detect other flavors of IE too. But for now, this will suffice.

Thanks for the great tool! I look forward to baking this deeper into the architecture of more projects.

SlexAxton commented 13 years ago

Even though you don't need front.js until after dom ready, doesn't mean you shouldn't load the file immediately. This is where you'll have nice performance gains.

yepnope is an asynchronous parallel script loader - The parallel part means that you can load all of your scripts at once. So you are welcome to load front.js later on, but you are only artificially extending the time that it takes for that script to run. This would probably end up being even slower than a normal script tag, because it isn't injected until after the dom is ready, whereas most scripts would be a part of the dom. That's all I meant by that. Certainly it will continue to work.

SlexAxton commented 13 years ago

I guess the key thing to realize is that you can load a script at a different time than it executes. If you wait until you have to execute a script to load it, then you put yourself in a spot of extra time, if it's already loaded, then you get immediate execution, which is awesome.

mkoistinen commented 13 years ago

Thanks, Slex, you're right of course and I do know this.

Since I've posted the above, I have removed the $(function() {...}) from my complete block as I had deemed it safe to start loading front.js anytime after jQuery. My reasoning before this change was based around not loading anything until the page is visually complete so as not to delay anything loading that is required for the visual "ready". Since front.js is the last thing loaded, I can safely assume that at worse, it may download in parallel with some of images, but would never block them. Currently this whole visually page loads in about 500ms from a server on the other side of the Atlantic, which is fantastic for this site. Another 2-3 seconds of JS will continue to execute preloading more content that is offscreen and behaviours that aren't required right away. I'm quite pleased with this, and Yepnope is a nice part of this, thank you.

Beyond that, I don't think I should be loading any of front.js until I am certain that jQuery is available, as everything in there is within self-running closures that require jQuery as parameter. I suppose I could preload it, but then I would need the preload! prefix now wouldn't I? =)

My main focus for this thread, however, is loading DD_belatedPNG in the right circumstances. And, while I have a workable solution now, I was hoping to convince either the folks at Yepnope or Modernizr to include IE-detection in their respective libraries. I can easily write my own (or just use the cond. comments as per above), but I'm not building a single webpage, but a whole, very large website, and I'm looking after getting the right mix of COTS components that I know will continue to be maintained and don't require a lot of customization over the coming months/years.

Modernizr, the only thing I want loaded in < head > (as it was intended) is where IE detection should reside. Yepnope already has this capability, but sadly as a plugin, which is not included in Modernizr, nor is it an installable option. Darn the luck.

SlexAxton commented 13 years ago

Got it, sorry got derailed.

There should be no issue adding prefixes to modernizr.load.

Here's how the Modernizr build tool works

  1. Include Modernizr (with correct options)
  2. Include yepnope 1.0.0 exactly
  3. Add Modernizr.load=function(){yepnope.apply(window,[].slice.call(arguments,0))};

So quite literally, there is nothing different about Modernizr.load than yepnope, because it calls yepnope in it. (yepnope is also leaked globally (on purpose)).

I just tested to see if prefixes loaded after Modernizr.load was aliased and they still work fine. (though you would add them to the yepnope object, NOT to the modernizr.load object).

After a modernizr build with modernizr.load try this:

yepnope.addPrefix( 'alert', function ( r ) { alert( 'works!' ); return r; });

Modernizr.load( 'alert!something/that/exists.js' );

This alerts just dandy. I've tested this with the ie prefix and it seems to work as intended.

mkoistinen commented 13 years ago

I just tried it all again, and it is working fine. My trouble before was that after my minification process, there was no preceding semicolon before the plugin's closure. I should have paid more attention before. Thanks!