Add load error capturing for callback.

walmartlabs / little-loader

A lightweight, IE8+ JavaScript loader

MIT License

370 stars 28 forks source link

Add load error capturing for callback. #21

Closed ryan-roemer closed 8 years ago

ryan-roemer commented 8 years ago

Holy moly, it works!!!

Fixes #15

Use script.onerror for IE9+
Use horrible hack for IE8
Add unit and functional tests
Update documentation and byte size

/cc @exogen @PeoB @aisapatino @baer

ryan-roemer commented 8 years ago

Travis hasn't picked up the PR yet. A SO comment says "close and reopen" can force a build, so doing that.

http://stackoverflow.com/questions/19589260/force-travis-to-build-a-specific-pull-request-or-commit-sha-that-it-missed

ryan-roemer commented 8 years ago

Weird, that worked. Travis is a 'runnin.

baer commented 8 years ago

Wowza - nice work!

exogen commented 8 years ago

Going over this in more detail. In general though I'd be very cautious about making such big changes without having a good test case first. I'm almost certain that most of the incorrect loaders would pass our current test suite here, and that none of the tests yet exercise the failure scenario (the one little-loader was made to pass, not "failure" as in onerror). So even if we're pretty sure that the non-error-case logic is kept the same, it would be nice to know for sure!

ryan-roemer commented 8 years ago

@exogen -- Teams need to be able to tell if a script fail right now. They're using onerror unreliably in VanillaJS code.

Can you open up a PR with a functional test to expose the issue or paste a diff here?

exogen commented 8 years ago

To follow up: this branch still passes the more brutal test I have in my repo, so it doesn't break anything, that's good! It sounds like @ryan-roemer is working on porting that test, as I'm mostly down for the count today. Still testing the new onerror handling.