Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix module definition and load sequencing #111

Merged
merged 2 commits into from
May 1, 2014
Merged

Conversation

kryptt
Copy link

@kryptt kryptt commented May 1, 2014

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

@jmdobry
Copy link
Owner

jmdobry commented May 1, 2014

@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
Copy link
Owner

jmdobry commented May 1, 2014

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

@kryptt
Copy link
Author

kryptt commented May 1, 2014

@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
Copy link
Author

kryptt commented May 1, 2014

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...

marcoslin/angularAMD#3

@jmdobry
Copy link
Owner

jmdobry commented May 1, 2014

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

@kryptt
Copy link
Author

kryptt commented May 1, 2014

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
Copy link
Author

kryptt commented May 1, 2014

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
Copy link
Owner

jmdobry commented May 1, 2014

I was planning on merging your pull request later today.

jmdobry added a commit that referenced this pull request May 1, 2014
Fix module definition and load sequencing
@jmdobry jmdobry merged commit 9dea799 into jmdobry:v2 May 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants