search

jmdobry / angular-cache

angular-cache is a very useful replacement for the Angular 1 $cacheFactory.
http://jmdobry.github.io/angular-cache
MIT License
1.39k stars 156 forks source link

Fix module definition and load sequencing #111

Closed kryptt closed 10 years ago

kryptt commented 10 years ago

angular-cache 2.3.3 refuses to load with angular 1.2.16 the way modules are accessed has been changed since then.

jmdobry commented 10 years ago

@kryptt How has the way modules are accessed been changed in angular 1.2.x? I can't find anything in angular's changelog that would indicate a breaking change.

jmdobry commented 10 years ago

@kryptt Here is a plunker using angular-cache 2.3.3 and angular 1.2.16. It seems to work fine.

kryptt commented 10 years ago

@jmdobry You're right, it's only happening under my own setup.

I'm loading things through requireJS and it could be related to this somehow.

kryptt commented 10 years ago

I worked on the plunker and made things load through requireJS and the error still doesn't show up Apparently it is due to an interaction with this other project:

angularAMD: https://github.com/marcoslin/angularAMD

Here is a plunker that reproduces the problem I am encountering: http://plnkr.co/edit/A1jve85Dl3Gc1llIKPA5

Apparently it's issue #3 for that project...

https://github.com/marcoslin/angularAMD/issues/3

jmdobry commented 10 years ago

@kryptt So this pull request is unnecessary and you'll wait for marcoslin/angularAMD#3 to be fixed or submit a PR there?

kryptt commented 10 years ago

Yes, it is unnecessary.

It's up to you whether or not to keep the semantics in this PR.

I'll try to make some time to look further into marcoslin/angularAMD#3 .. thanks, for the support and quick reply!! :D

kryptt commented 10 years ago

If I were to make a case for this semantic then:

It is theoretically faster to use the result of registration right away, as opposed to discarding it, and then making a new search among the registered modules...

jmdobry commented 10 years ago

I was planning on merging your pull request later today.