Deprecate transition methods of Controller and Route

[jelhan]

Rendered

[rwjblue]

The router is known to be bad documented and underspecified

😭 this is painful to read but... it is not wrong 😩

[rwjblue]

Overall, I'm 👍 here. I think we should also update the prose to account for ember-engines, which has a custom method for this stuff (transitionToExternal).

[jelhan]

Hello @rwjblue. Thanks a lot for your review.

I think we should also update the prose to account for ember-engines, which has a custom method for this stuff (transitionToExternal).

To be honest I don't have much experience with Ember Engines. So not sure what would be the best transition path for transitionToExternal and replaceWithExternal methods injected to Route and transitionToExternalRoute method injected into Controller by Ember Engines are.

Deprecating these methods as well seems to fit best with the attend of this RFC. But it seems as if something similar to router service is still missing for Ember Engines: ember-engines/ember-engines#587 There is an pull request to implement a ExternalRouterService. This one could be serve as a transition target. But it's not merged yet: ember-engines/ember-engines#669

Do you think this RFC must cover Ember Engines? Or would it be enough to add something like this to drawbacks section:

Ember Engines injects Route#transitionToExternal, Route#replaceWithExternal and Controller#transitionToExternalRoute methods. These methods allow the consumer to transition between routes external to the engine. Ember Engines does not provide a replacement similar to router service yet. Even though it's under active development. The APIs recommended by Ember and provided by Ember Engines will not align very well until Ember Engines is ready to also deprecate Route#transitionToExternal, Route#replaceWithExternal and Controller#transitionToExternalRoute.

[rwjblue]

Do you think this RFC must cover Ember Engines?

Ya, I do think we should mention it as well, but I agree that the details are not this specific RFCs problem. Maybe you could add a small blurb to mention ember-enginess transitionToExternal and state that that will be deprecated as well once ember-engines support for the router service is available?

[jelhan]

@rwjblue I added some paragraphs discussing how this RFC would affect Ember Engines in ce6e2a2. Let me know if you think that it needs further clarification.

[rwjblue]

Thank you @jelhan!

[kurkowski]

This issue describes a bug with the Router service's transitionTo method. As a workaround for the issue, you can use the Route/Controller's transitionTo. Is this bug going to be fixed as part of this deprecation? Otherwise, we are stuck between using a deprecated API or having a buggy experience.

[tomdale]

Yes, we need to fix this before the deprecation goes in. @rwjblue is actively working on this per @krisselden.

[vitch]

I came here to mention something similar. There have been multiple cases that we have run into where it's not possible to replace a use of route.transitionTo with the RouterService's transitionTo. I'll have to go digging to find the details but they have been related to query params.

I remember at the time I last ran into it reading the RouterService RFC and being convinced by the "Query Parameter Semantics" section that this was by design:

Today, queryParams impose unnecessarily high cost because we cannot generate URLs or determine if a link is active without taking into account the default values of query parameters. Determining their default values is expensive, because it involves instantiating the corresponding controller, even in cases where we will never visit its route.
Therefore, the queryParams argument to the new urlFor, transitionTo, replaceWith, and isActive methods defined in this document will behave differently.

I take that to mean that RouterService.transitionTo will never be a drop in replacement for router.transitionTo. If this is true then should these edge cases be acknowledged in the "Transition path" section of this RFC?

[tomdale]

We discussed this in the core team meeting today and we've decided to move it into Final Comment Period. Thanks for submitting this RFC, @jelhan!

[locks]

Some frontmatter house keeping.

addressed

[jenweber]

Thank you for the “How we teach this” audit of learning materials. I think this depreciation will be helpful because we don’t need to teach as many things to beginners. That seems like a win to me.

[ijlee2]

@jelhan Thank you for working on writing the proposal. From the perspectives of a new learner and a contributor to the Ember Guides, I agree that it will be good to simplify how a route transition is to occur from controllers and routes.

Today, I was able to spend half an hour on updating the syntax this.transitionTo in routes and this.TransitionToRoute in controllers to this.route.transitionTo in a production app. I think most cases were simple (transition to an access denied page) that I don't expect timing issues that you described in the RFC to occur in this app.

I did encounter several instances of self.transitionTo and self.transitionToRoute, because the app doesn't fully utilize arrow functions yet. I think it might be nice if the person who will work on the codemod can:

1. Look for the patterns transitionTo and transitionToRoute (instead of this.transitionTo and this.transitionToRoute)
2. Either:
   a. Always replace transitionTo and transitionToRoute with router.transitionTo; or,
   b. Replace only when the word that appears before transitionTo and transitionToRoute is this.. Output a message if the word that appears before isn't this. so that a developer can check and manually replace the line.

[locks]

@ijlee2 appreciate you voicing your concern. That is certainly something that whoever implements the codemod should keep in mind.

We discussed this RFC at this week's framework team meeting and decided to move the RFC forward.
Thank you @jelhan and all commenters for participating in the process!