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

removing module.register #63

Merged
merged 1 commit into from
Jun 13, 2014
Merged

Conversation

matjaz
Copy link
Contributor

@matjaz matjaz commented Jun 11, 2014

No need separating registering lazy loaded controllers/services/...

Lazy loaded code should be the same as sync or async loaded.
Controllers, services, filters,... should run without angularAMD.

Based on: https://github.com/matjaz/ng-1.x-async-hack

@marcoslin
Copy link
Owner

@matjaz thanks but I cannot merge this pull request as it represent breaking change for existing user. We should keep app.register for backward compatibility.

About your comment: "Lazy loaded code should be the same as sync or async loaded", I was thinking to implement it in a way that extend that statement to support registering modules before bootstrap. Essentially, the way going forward is to do:

angularAMD.controller(...)

This way, angularAMD would cache the requests and apply to provider when .bootstrapis called.

Feedback?

@matjaz
Copy link
Contributor Author

matjaz commented Jun 11, 2014

ok. you can add app.register as an alias for /bc.

My point of view is angularAMD should be used only for angularAMD.route(). Other code should not rely on it and it should work without it. Only with requirejs or as a plain standalone angular module.

@marcoslin
Copy link
Owner

@matijaz I need to give this some more thoughts. I was planning to do exactly the opposite of your suggestion, i.e., to increase the use angularAMD through out the project.

In my own projects, I often have library that initially only used by few modules and then it's use is extended. In this case, I would ideally wanna move this module to load as a dependency to app instead of individual modules. The only way to accomplish this move transparently is to do something like angularAMD.service(...).

Having said that, my suggestion can be easily done in conjunction to your proposal. I just need to digest such change and see if it create more confusion for normal users.

It's just a bad week for me unfortunately.

@marcoslin marcoslin added this to the v0.1.2 milestone Jun 13, 2014
marcoslin added a commit that referenced this pull request Jun 13, 2014
@marcoslin marcoslin merged commit 54b5ec2 into marcoslin:master Jun 13, 2014
@marcoslin marcoslin modified the milestones: v0.2.0, v0.1.2 Jun 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants