Easier CDN fallback support and IE 6-8 fix #48
Conversation
use case example:
```js
$script(cdnSrc.Modernizr, 'Modernizr', function() {
//code
}, cdnFallback);
function cdnFallback(depsNotFound) {
var i = depsNotFound.length;
while (i--) {
console.log('cdnFallback '+depsNotFound[i]);
$script(cdnFallbackSrc[depsNotFound[i]], depsNotFound[i]);
}
}
```
Currently to allow fallback the documentation suggest something along the lines of
```js
$script(cdnSrc.Modernizr, 'Modernizr');
$script.ready('Modernizr', function() {
console.log('Modernizr ready');
modernizrFallback(Modernizr);
bootstrap();
}, cdnFallback);
```
However this will load the both the CDN file and the fallback file.
This commit separates out the onerror callbacks to allow
calling a fallback function.
Array.forEach is not supported in IE 6-8. `while(i--)` is the fastest way to go through a loop.
|
right on. can you add examples to the README and adhere to the style of the code (or if you'd rather, i can point out the nits) |
2. Added CDN fallback example (More examples can be found at https://github.com/willfarrell/angular-io/blob/master/src/scripts/async.js)
- Compiled code
|
Done |
|
This will really be useful, but could you add it into the README separation between the different examples, I would like to see a more complete example of different cdn fallbacks. And can this be merged? |
|
BTW, I tested your pull request, and I'm finding that depsNotFound is always the full id array. There's no differentiation between which exact dependency failed. It would be great if there was some form of distinction. That way you could use a single call to $script and depending on which one failed, it will make the appropriate fallback. Right now you have to make multiple calls to $script. |
- script.js update to match current repo branch + support for `script('src', function(){}, fallback)`
- testing added
|
@CMCDragonkai A small clean up to the README is uploaded You are correct there are multiple calls to $script, please create this as a unique issue and if you can a pull request (Check out $script.ready). Note the browser will only make one http call per unique url, so not too critical. |
|
k, will have a look at this now and try to sort out the conflict |
|
Exactly what I was looking but 1 Issue fix that worked for me: Also 1 thing unrelated when i minify the source after the change Also the fallback will not work in IE8 but it does work in IE10. |
|
@esnellman The output you provided is correct based on your code. You have two/three jquery ready callbacks set ( The CDN fallback feature was intended for falling back to a file stored on your own server (but it will still handle falling back to another CDN of course). Having this setup is faster than falling back to another CDN. Every time you include a resource from a different URL an additional DNS lookup is required (they are costly). Generally I only use a remote CDN if I have three or more popular resources that need to be loaded. $script('//cdnjs.cloudflare.com/ajax/libs/jquery/1.10.2/jquery.min.js.NOTFOUND', 'jquery', function() {
console.log('cloudflare worked');
console.log('jquery is ready!');
}, function() {
console.log('cloudflare failed');
$script('//ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js', 'jquery', function() {
console.log('googleapis worked');
}, function() {
console.log('googleapis failed');
});
});I'll check out your suggested change to PS Closure, as far I remember from what my friends at Google told me, is old and no longer worked on. Everyone uses |
|
I got 3-5 so far, bootstrap, fontawesome, jquery, respond, html5shiv Will make a note to use uglify. simplified issue: |
|
Perhaps the README should include some motivation. One of the most common CDN's is S3 + Cloudfront. However, CORS is not well supported by S3 + Cloudfront due to this issue: https://forums.aws.amazon.com/thread.jspa?threadID=114646 There is a workaround available, but it gets broken if a spider or "hacker" downloads a file without sending the origin header. Cloudfront then caches these invalid response headers and sends them to everybody behind that edge location resulting in a completely broken application. We've had significant downtime with some clients because of this issue. This PR in theory would have allowed us to continue to use $script without losing the benefits of a CDN. If this issue was called out in the README perhaps we would have known to actually set things up this way rather than learning the lesson the hard & expensive way. |
|
+1 |
|
I feel the industry has moved past the need for this PR. Any objections to me closing this? |
See commit comments for details.