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

[DOC] Mark replaceWith and replaceRoute as deprecated #19775

Conversation

bertdeblock
Copy link
Member

@bertdeblock bertdeblock commented Oct 7, 2021

Related to emberjs/rfcs#674.

@btecu
Copy link
Contributor

btecu commented Oct 7, 2021

Before merging this change, it would be good to see some of the issues linked here addressed #19609.

Currently, on a route this.transitionTo and this.router.transitionTo might not work the same, so that should be addressed before this deprecation.

@bertdeblock
Copy link
Member Author

I'm not really following. The linked RFC has already been merged and implemented a while ago. Those methods are already deprecated. This just adds the @deprecated label where it was forgotten.

@@ -162,7 +162,7 @@ ControllerMixin.reopen({
@method transitionToRoute
@return {Transition} the transition object associated with this
attempted transition
@deprecated Use transitionTo from the Router service instead.
@deprecated Use `transitionTo` from the Router service instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the backticks here actually work, we have to confirm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we show these messages? It seems they are not displayed in the docs. Do you want me to remove the backticks?

@bertdeblock bertdeblock force-pushed the mark-replaceWith-and-replaceRoute-as-deprecated branch from c850386 to 50359b0 Compare October 22, 2021 16:02
@locks locks merged commit 99791ad into emberjs:master Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants