Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

[TO DISCUSS] Provide a method to get a list of all defined controllers #13951

Closed
Vadorequest opened this issue Feb 5, 2016 · 15 comments
Closed

Comments

@Vadorequest
Copy link
Contributor

Problem

There is no way to detect whether a controller has been defined or not without instanciate it, which isn't a good solution. See discussion on SO: https://stackoverflow.com/questions/19734565/how-to-check-if-an-angularjs-controller-has-been-defined

Use case

Say that we have an angular app which is a generic product which also deals with specific content based on a customer. Say that we have generic controllers that are use by default, except when a specific controller with the customer suffix exists and we want to use that controller, but only if it exists. Otherwise we would use the generic controller. (Basically a controller fallback)

In this case we need to check if the specific controller exists and if it doesn't then use the generic one.

Example

angular.module('app').config ($stateProvider, ClientServiceProvider) ->
  _ClientService = ClientServiceProvider.$get()

    $stateProvider
    .state 'app',
        controller: _ClientService.resolveClientCtrl 'AppCtrl'

The resolveClientCtrl method would check whether or not a specific controller exists or use the provided controller. (this is an example of use, for a specific need)

Proposition 1

Angular already has a list of controllers used internally which aren't shared out of its scope. What we could do is simply add a method that provides the list of defined controller as an array of strings.

A solution close to this has been proposed here on SO: http://stackoverflow.com/a/19736852/2391795

Proposition 2

We could also just add a method which doesn't provide the list of controllers but check whether a controller exists or not.


This has been asked two years ago and has more than 5k views in one SO question. I believe this should be in angular "core".

If one of those propositions is accepted (or both) then I'll make a PR.

@Narretz
Copy link
Contributor

Narretz commented Feb 5, 2016

Thanks for the detailed writeup. What I wonder:
Is there a specific reason why you need different controllers? Couldn't you delegate the task of deciding to a simple controller, which then calls a service based on the input / state?

Adding a has method would probably the best way to resolve this issue. But I'm not convinced that there's a pressing need.

@Vadorequest
Copy link
Contributor Author

Well, first I think that if so many people have viewed that question it means that it's not just for my specific use but because there is a greater need. I just gave one example of use.

And yes. We don't want to check inside the same controller whether it is client A, B or C. Because we could have hundreds of them and the code would become unmaintainable. So we need to separate those.

A has method would do the trick for my specific need indeed. That would be enough. I'm trying to figure out if there is use case where we would need a complete list of all controllers name, like in an array. That would allow more things actually, that would offer more possibilities.

@Narretz
Copy link
Contributor

Narretz commented Feb 5, 2016

What's the difference between checking in the controller or in the stateProvider which functions are needed? Why does checking inside the controller become unmaintainable while checking before the controller instantiated does not? In my mind it's just a minimal difference.

@Vadorequest
Copy link
Contributor Author

Because when you do it in the routes, it's just a wrapper. Each controller can have its separated logic, display different forms, inputs, data. Make different ajax call by calling different services.

If you call one controller only which does all the checking, it may be fine for simple scenarii. But it's gonna get overcomplicated as soon as you have to deal with different logic and so on. There will be a lot of if checks and you won't be able to understand the workflow easily.

And, using a wrapper, you can still use inheritance between controllers if you want, and share some logic the way you want it. It's powerful.

@gkalpak
Copy link
Member

gkalpak commented Feb 6, 2016

In the scenario you describe, there will not be just different controllers (or at least there shouldn't), but there will also be different templates and possibly different services or service wrappers (if not filters, directives etc).

I would bet there are much better (and more robust) ways to solve this.
A few off the top of my head:

  1. If a user/customer should have access to non-default features, load a module which overwrites the needed services/controllers/templates etc. Then, you don't have to check at runtime if a special controllers exists for the user, you just load AppCtrl and the correct controller will be loaded.
  2. Maintain your own list of user-specific controllers and check against that list before loading the controller.
  3. Make the actual controller a thin wrapper that delegates everything to a service. Instead of user-specific controllers, have user-specific services and determine the correct service to delegate to from inside the controller (there you can use $injector.has() for example).

@Vadorequest
Copy link
Contributor Author

There are indeed different templates, the same way there are different controllers;

But I don't see how using a different module would allow us to fallback to the generic module when there isn't anything specific to do. There are a lot of generic code and we won't override everything, so we need to fallback most of the time.

@gkalpak
Copy link
Member

gkalpak commented Feb 6, 2016

You can always load the generic module (containing the fallback stuff) and loading the user-specific module (which overwrites/decorates the needed components) in certain cases.
I can't be more specific without more context, though.

@Vadorequest
Copy link
Contributor Author

Anyway, that's not really the point of this question in the first place. Is it?
So, should i make a PR adding a .has method?

@gkalpak
Copy link
Member

gkalpak commented Feb 6, 2016

Although it's very easy to add and will have minimal overhead, I don't feel it promotes any good practices, so I wouldn't be too keen on adding it.
I don't feel too strongly about it though, so let's see if anyone likes the idea 😃

@petebacondarwin
Copy link
Contributor

@Vadorequest - is it your requirement that in some application for, say ClientA, that whenever you ask for some controller, say MyController, it should actually create MyControllerClientA, if it is defined and MyController otherwise?

Are there instances where you would want to still get the original MyController, even if MyControllerClientA is defined in an app?

@petebacondarwin petebacondarwin self-assigned this Feb 22, 2016
@petebacondarwin petebacondarwin modified the milestones: Purgatory, Ice Box Feb 22, 2016
@Vadorequest
Copy link
Contributor Author

@petebacondarwin Yes we do. Because the Controller MyControllerClientA may inherit from MyController.

@petebacondarwin
Copy link
Contributor

That is a shame because I was really keen on the idea of simply providing an extra module that overrode the generic controllers.

@petebacondarwin
Copy link
Contributor

I guess that, given we have an $injector.has() then we already have a precedent.
Given that it is such a simple addition, I am OK with adding this.

@petebacondarwin petebacondarwin modified the milestones: 1.5.x, Purgatory Feb 22, 2016
@petebacondarwin
Copy link
Contributor

@Vadorequest feel free to submit a PR.

@Vadorequest
Copy link
Contributor Author

Duly noted!
Thank you, I'll do it ASAP.

Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
…ss, to check the existence of a given controller. Fixes angular#13951
Vadorequest added a commit to Vadorequest/angular.js that referenced this issue Feb 23, 2016
…ss, to check the existence of a given controller. Fixes angular#13951
gkalpak pushed a commit that referenced this issue Feb 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants