Controller Injection Deprecation

[mehulkar]

rendered

[btecu]

While this has a noble cause, I'd rather not introduce churn if the eventual goal is to eliminate controllers anyway.

[mehulkar]

@btecu are you worried about churn in Ember or churn in your app? Are you using controller injection today? To me it seemed like a pretty isolated refactor that would have to give apps the chance to move towards a happy path. My guess is that it won’t affect many users but for the few that it does, it will be a good place to start.

[btecu]

I was referring to the one in applications. I do work on at least one application that uses controller injection and I'd rather not touch that until there's something better to replace (controllers) with.

[mehulkar]

@btecu I can understand your concern and thanks for the feedback! The controller removal journey will likely include untangling controller dependencies as a first step for you regardless of when you do it. Because this is a deprecation not a removal, the only risk is the (probably unlikely?) event that the next major release of Ember keeps controllers, but removes controller injection. I'm not sure that situation is worth hedging against. Would you like to see the RFC address this situation?

[btecu]

Thanks for responding @mehulkar.
If having controller injection was blocking tree shaking or any other feature I'd understand but it's not and right now we're theorizing what and when will get removed.

The feature deprecation vs removal has become kind of an excuse, as we all know 95% of the developers that care about the projects they work on are not going to stand having deprecations in their applications.

[Gaurav0]

The feature deprecation vs removal has become kind of an excuse, as we all know 95% of the developers that care about the projects they work on are not going to stand having deprecations in their applications.

I don't understand this philosophy, having deprecations in real applications is pretty much unavoidable. I'm not going to rush and upgrade every addon I use in response to a new deprecation or wait to upgrade indefinitely. This is not realistic.

I would much prefer to have my controllers deprecated in pieces than all at once.

[bendemboski]

I use controller injection to share functionality on a root controller (my-route) with leaf controllers (my-route.index, my-route.page1, etc.). I could put it in a service, but services are global and this functionality only makes sense in my-route.* routes, so from a code organization/architecture standpoint I much prefer to put it on an object that is not application-globally-scoped.

My belief/hope is that when controllers are deprecated, there will be a clean story for non-global route-scoped state and functionality. If I go and change all my controller injections to global services or .lookup()s, they will not be syntactically identifiable anymore, and it will be more difficult for me to translate that route-scoped state/functionality into whatever it looks like in the post-controller world. It seems like I'd be losing information that could be very helpful when it comes time to move away from controllers, and I'm having trouble seeing the benefits that offset this.

If almost nobody is using controller injection, then there's extremely limited value in deprecating it. If more people are using it, then we should have a clean story for replacing what they're doing with it that isn't "shove it into some other pattern that already exists but isn't necessarily targeted towards your use case" because then the actual data/architecture pattern gets obfuscated and becomes harder to port to the post-controller world whenever that arrives.

So my 2 cents is to not deprecate controller injection until/unless we have a clear pattern to tell people to replace it with, and I don't think another at-least-as-good pattern currently exists for route-scoped functionality and state, so I think deprecating this is premature. Maybe first we should focus on route-scoped services and then come back to this once its functionality has actually been replaced?

[lolmaus]

I've seen many projects that rely on controller injection.

This technique has no negative consequences, no overhead, thanks to controllers being singleton and, unlike components, never destroyed throughout the app lifecycle.

Eliminating controller injection will force users to create more ad-hoc services. The code that fits naturally into a controller now has to be moved to an arbitrary service, whose only purpose is to let a second controller access the first one.

This increases code complexity and maintenance burden with zero practical benefits, except for fuelling hatred towards controllers just for the sake of it.

---

An example use case.

A nested controller contains controls, whose state needs to survive page transitions. I. e. when I visit the page for the second time, I do not want to adjust all the controls for the second time. I want to continue where I left off.

Since those controls are only used on that page, keeping relevant code in the controller is very natural and practical. When a developer wants to find the relevant code, the controller will be the first place they will be looking in. That's the controller's main responsibility.

But then there's a requirement for a parent template to render a status indicator based on the state of the controls. Using a controller injection is a simple, straightforward and very efficient way of achieving this.

Without controller injections, you have three options:

• Move the logic to the parent controller. This is terrible because the logic is no longer contained where it naturally belongs. If there are multiple child controllers like that, the parent controller would grow in size and become a pain to maintain.
• Create an ad-hoc service. You'd probably name it the same as the controller with controls, because it has no other purpose than to store controller's logic. The extra entity provides no benefit, it's just a shim to connect controllers together. The more cases like this, the more unnecessary singletons you'll have to instantiate.
• Use controllerFor or owner.lookup. That's the same as an injection, except that it requires more boilerplate and is error-prone.

I just don't understand how any of this is better.

---

Controllers are fine. I really don't see why everyone hates them so much.

I guess it's OK to hate them if you feel like it (unless you're just trying to jump on the controller hatred hype train in order to author an RFC), but PLEASE do not attempt crippling controllers before you provide a functional, efficient and ergonomic replacement. Web development is hard enough already.

[pzuraq]

We discussed this RFC at the core team meeting today, and came to the decision that while we do want to deprecate this functionality in the future, it would be premature to do it at this time for the following reasons:

1. Deprecating controller injections now, without deprecating controllers as a whole and providing a cohesive alternative, would likely lead to churn. Users would have to convert to an intermediate solution now, and then convert again to whatever the full replacement for controllers would end up being.

2. Query parameters often need to be accessed from other controllers, and cannot be easily extracted to a service due to many edge cases in their behaviors. In this case, there isn't an alternative yet at all.

We are moving this RFC into FCP to close for these reasons, but we are 100% on board with this direction in general. The best way to make progress here, for anyone interested, would be to work on RFCs for alternatives to controllers, and in particular alternatives to query parameters as a whole since they are one of the larger blockers for deprecating controllers. There are a few RFCs out there that have proposed some ideas in this space, but we think that they still need time to bake, with extra attention to the incremental migration path from the existing controller/QP setup to the new one.

[mehulkar]

(2) is a valid reason that I can actually agree with it. I had overlooked this use case when I was working on this RFC because I always default to defining query params at the ApplicationController level.

Thanks for taking the time to provide feedback everyone. Closing this down as it doesn't seem to have enough support from the community or the core team.

[bendemboski]

Thanks for your work on this @mehulkar! I disagreed with it, but also very much appreciate the effort you put into writing it up -- important to have well-written RFCs to move discussion and thinking forward!