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

fix(ngController): allow bound constructor fns as controllers #10790

Closed

Conversation

pkozlowski-opensource
Copy link
Member

Fixes #10784

@caitp
Copy link
Contributor

caitp commented Jan 17, 2015

hmm, it's really too bad we don't have Reflect.IsConstructor() yet, that would make this a lot easier.

@caitp
Copy link
Contributor

caitp commented Jan 18, 2015

actually you know what, I think this is still a kind of dumb idea, since the bound function won't have the expected prototype. It would be like giving people a footgun to shoot themselves with.

I apologize for changing my mind about this twice today, but unfortunately we have no way of getting at the "real" prototype from JS :(

@pkozlowski-opensource
Copy link
Member Author

@caitp it is OK to change ones mind if in the end it leads to better framework! I understand your position and I don't have any more arguments apart from the ones presented already:

  • bound constructor functions are special animals that are half-constructors (you can new them but those won't have a prototype) - so it is hard to arbitrary say if it is a proper constructor or not - we are in the grey are here...
  • people using bound constructor functions should be aware of the fact that prototypes are not available - this is how it works in vanilla JS
  • I'm not sure how common it is to use both prototypal inheritance for controllers
  • it used to work in 1.2.x
  • it is a trivial fix

But as I've stated before I don't feel strongly about this one as I don't need it personally. I also understand your will to "protect the innocent" and don't give people tools that they can use to hurt themselves.

To sum up: I've got nothing more to say / do when it comes to this issue so if you @caitp or @petebacondarwin got a definitive opinion on this one please close / merge this PR / associated issue.

@petebacondarwin
Copy link
Contributor

We should land this, since it was available throughout for 1.2 and no one managed to shoot themselves in the foot so far; so it is probably a very unlikely thing to happen. In the meantime people do want to be able to create bound controllers in their applications and the use cases provided are reasonable.

We should put a disclaimer in the documentation for ngController and module.controller to highlight that bound controllers will not have the expected prototype.

Can you do that and then merge @pkozlowski-opensource ?

@thinkingmedia
Copy link
Contributor

-1 for this change. I can not think of one case where a programmer would need to do this. It's more likely they're doing something they shouldn't. I would hate to see this introduce modules that become popular but are full of odd controllers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bound constructor functions can't be used as controllers
5 participants