From 7aef9be4cdd67b857c7349512b3090038ab8eeb3 Mon Sep 17 00:00:00 2001 From: Ruslan Hrabovyi Date: Wed, 14 Nov 2018 00:40:34 +0200 Subject: [PATCH] [@ember/routing] Fix redirect hook Prior to this commit `redirect()` hook was mistakenly typed and documented the same way as `refresh()`. It's now fixed for v2 and v3 now. While v3 is provided with a positive and a negative test cases, v2 is only provided with a positive one. This is because v2 uses typescript of version 2.4 which is not able to catch the failing test case. --- types/ember/v2/index.d.ts | 32 ++++++++++++++++++------------ types/ember/v2/test/route.ts | 8 ++++++++ types/ember__routing/route.d.ts | 32 ++++++++++++++++++------------ types/ember__routing/test/route.ts | 16 +++++++++++++++ 4 files changed, 62 insertions(+), 26 deletions(-) diff --git a/types/ember/v2/index.d.ts b/types/ember/v2/index.d.ts index 93babaa0a7..54b6d2e4a9 100755 --- a/types/ember/v2/index.d.ts +++ b/types/ember/v2/index.d.ts @@ -1895,20 +1895,26 @@ declare module 'ember' { paramsFor(name: string): {}; /** - * Refresh the model on this route and any child routes, firing the - * `beforeModel`, `model`, and `afterModel` hooks in a similar fashion - * to how routes are entered when transitioning in from other route. - * The current route params (e.g. `article_id`) will be passed in - * to the respective model hooks, and if a different model is returned, - * `setupController` and associated route hooks will re-fire as well. - * An example usage of this method is re-querying the server for the - * latest information using the same parameters as when the route - * was first entered. - * Note that this will cause `model` hooks to fire even on routes - * that were provided a model object when the route was initially - * entered. + * A hook you can implement to optionally redirect to another route. + * + * If you call `this.transitionTo` from inside of this hook, this route + * will not be entered in favor of the other hook. + * + * `redirect` and `afterModel` behave very similarly and are + * called almost at the same time, but they have an important + * distinction in the case that, from one of these hooks, a + * redirect into a child route of this route occurs: redirects + * from `afterModel` essentially invalidate the current attempt + * to enter this route, and will result in this route's `beforeModel`, + * `model`, and `afterModel` hooks being fired again within + * the new, redirecting transition. Redirects that occur within + * the `redirect` hook, on the other hand, will _not_ cause + * these hooks to be fired again the second time around; in + * other words, by the time the `redirect` hook has been called, + * both the resolved model and attempted entry into this route + * are considered to be fully validated. */ - redirect(): Transition; + redirect(model: {}, transition: Transition): void; /** * Refresh the model on this route and any child routes, firing the diff --git a/types/ember/v2/test/route.ts b/types/ember/v2/test/route.ts index 84e432dbd3..9ce18b410c 100755 --- a/types/ember/v2/test/route.ts +++ b/types/ember/v2/test/route.ts @@ -86,6 +86,14 @@ Route.extend({ }, }); +class RedirectRoute extends Route { + redirect(model: {}, a: Ember.Transition) { + if (!model) { + this.transitionTo('there'); + } + } +} + class RouteUsingClass extends Route.extend({ randomProperty: 'the .extend + extends bit type-checks properly', }) { diff --git a/types/ember__routing/route.d.ts b/types/ember__routing/route.d.ts index 118bd1f1b5..30c9654be9 100644 --- a/types/ember__routing/route.d.ts +++ b/types/ember__routing/route.d.ts @@ -79,20 +79,26 @@ export default class Route extends EmberObject.extend(ActionHandler, Evented) { paramsFor(name: string): {}; /** - * Refresh the model on this route and any child routes, firing the - * `beforeModel`, `model`, and `afterModel` hooks in a similar fashion - * to how routes are entered when transitioning in from other route. - * The current route params (e.g. `article_id`) will be passed in - * to the respective model hooks, and if a different model is returned, - * `setupController` and associated route hooks will re-fire as well. - * An example usage of this method is re-querying the server for the - * latest information using the same parameters as when the route - * was first entered. - * Note that this will cause `model` hooks to fire even on routes - * that were provided a model object when the route was initially - * entered. + * A hook you can implement to optionally redirect to another route. + * + * If you call `this.transitionTo` from inside of this hook, this route + * will not be entered in favor of the other hook. + * + * `redirect` and `afterModel` behave very similarly and are + * called almost at the same time, but they have an important + * distinction in the case that, from one of these hooks, a + * redirect into a child route of this route occurs: redirects + * from `afterModel` essentially invalidate the current attempt + * to enter this route, and will result in this route's `beforeModel`, + * `model`, and `afterModel` hooks being fired again within + * the new, redirecting transition. Redirects that occur within + * the `redirect` hook, on the other hand, will _not_ cause + * these hooks to be fired again the second time around; in + * other words, by the time the `redirect` hook has been called, + * both the resolved model and attempted entry into this route + * are considered to be fully validated. */ - redirect(): Transition; + redirect(model: {}, transition: Transition): void; /** * Refresh the model on this route and any child routes, firing the diff --git a/types/ember__routing/test/route.ts b/types/ember__routing/test/route.ts index 7e2279d8c4..b347768301 100755 --- a/types/ember__routing/test/route.ts +++ b/types/ember__routing/test/route.ts @@ -80,6 +80,22 @@ Route.extend({ }, }); +class RedirectRoute extends Route { + redirect(model: {}, a: Transition) { + if (!model) { + this.transitionTo('there'); + } + } +} + +class InvalidRedirect extends Route { + redirect(model: {}, a: Transition, anOddArg: any) { // $ExpectError + if (!model) { + this.transitionTo('there'); + } + } +} + class ApplicationController extends Controller {} declare module '@ember/controller' { interface Registry {