Fix loader for decorator and component

[Narretz]

The changes seem very straightforward, since the custom loader is mainly copy / paste. I've additionally included a change for better error messages.

Question is, how do I test if this works? Can I run a version of the plugin locally?

[gkalpak]

I guess you could load your local copy as unpacked extension and test it, but there should be automatic tests in place. I remember there was a discussion some time ago about setting up unit-/integration-testing against actual apps, but I'm not sure if this ever moved forward.

[Narretz]

There seem to be some tests, but the build fails. I think we should just test this manually, and merge it.

[gkalpak]

One problem with this change is that it might behave strangely if the used version of angular does not support decorator() or component(). I.e. if someone is (incorrectly) trying to use .decorator() or .component() with a version of angular that does not have them, they should get an error, but they won't if they test it with Batarang enabled.

I am not sure it is worth fixing it though...

[gkalpak]

I tried this and it throws an error because isFunction() is not defined.

[gkalpak]

Now, identifierForController is not defined.

[gkalpak]

This is getting kind of out of control. There must be a better way, than defining a custom loader in Batarang that is mostly/only copy-paste.

@btford or anyone else, any idea why Batarang needs its own loader.js in the first place ?

[Narretz]

For compat in 1.2 and 1.3: https://github.com/angular/batarang/commit/6a0ae94f4df7f5dea3a10874f6f9be9603e353cc and the mentioned issues

I've tried to recreate the original error to see if a different change is possible, but I haven't been able to reproduce it.

[Narretz]

I have tested with various configurations, and the easiest way to provoke the original error that the loader is supposed to fix is to run angular with version 1.2.29, but use the the angular-loader file from >=1.3.0

So do npm install angular-loader And in hint.js change this line:

// require('./loader.js');
require('angular-loader');

Then the app should fail with a provider not found error. Essentially, the patch in the custom loader changes the behavior of loading the config block to match that of angular 1.3. The major work for this change in 1.3 was in the $injector, so it's indeed a strange workaround, which basically adds the BC into 1.2.

Regarding identifierForController, I assume that is in the angular closure, so we have no access to it if the loader is used separately. This should also be an issue if people use the loader for async module loading.

[gkalpak]

After reading https://github.com/angular/angular-hint/issues/39, I'm still not sure why Batarang needs a custom loader. If people use matching versions of loader and angular, shouldn't things just work fine ?

[petebacondarwin]

Now that we have refactored the loader in AngularJS 1.5.0 - we need to change this PR accordingly. See https://github.com/angular/angular.js/pull/13692

[Narretz]

I've updated the PR. @gkalpak Before there was a custom loader, batarang would require the default angular-loader, so that ngHint can hook into it before the module loader gets set up. So basically your app will use the loader from batarang.

[gkalpak]

LGTM

[gkalpak]

It's been a while since travis was last happy with batarang. Is it because of the old Chrome version ?

[SomeKittens]

Fantastic, thanks so much!

@gkalpak not sure, we have an issue open but I haven't looked into it much, just ran the tests manually.