diff --git a/packages/ember/tests/routing/router_service_test/transitionTo_test.js b/packages/ember/tests/routing/router_service_test/transitionTo_test.js index 0d0ab1db4cf..9d14fbe7cbc 100644 --- a/packages/ember/tests/routing/router_service_test/transitionTo_test.js +++ b/packages/ember/tests/routing/router_service_test/transitionTo_test.js @@ -439,5 +439,86 @@ moduleFor( assert.equal(this.routerService.get('currentURL'), '/?url_sort=a'); }); } + + async ['@test Redirecting to same route with query params from beforeModel works via transition']( + assert + ) { + assert.expect(2); + + this.add( + 'route:parent.child', + class extends Route { + @service + router; + + beforeModel(transition) { + if (!transition.to.queryParams.bar) { + return this.router.transitionTo('parent.child', { + queryParams: { bar: '1' }, + }); + } + } + } + ); + + this.add( + 'controller:parent.child', + Controller.extend({ + queryParams: ['bar'], + bar: null, + }) + ); + + await this.visit('/'); + await this.routerService.transitionTo('parent.child').followRedirects(); + + assert.equal( + this.routerService.get('currentURL'), + '/child?bar=1', + 'redirected to self with query param' + ); + + assert.equal(this.routerService.get('currentRouteName'), 'parent.child'); + } + + async ['@test Redirecting to same route with query params from beforeModel works via direct visit']( + assert + ) { + assert.expect(2); + + this.add( + 'route:parent.child', + class extends Route { + @service + router; + + beforeModel(transition) { + if (!transition.to.queryParams.bar) { + return this.router.transitionTo('parent.child', { + queryParams: { bar: '1' }, + }); + } + } + } + ); + + this.add( + 'controller:parent.child', + Controller.extend({ + queryParams: ['bar'], + bar: null, + }) + ); + + await this.visit('/child'); + + assert.equal( + this.routerService.get('currentURL'), + '/child?bar=1', + 'redirected to self with query param' + ); + + assert.equal(this.routerService.get('currentRouteName'), 'parent.child'); + } } ); diff --git a/packages/router_js/lib/router.ts b/packages/router_js/lib/router.ts index 0dd1300ae8c..89d05bcc43c 100644 --- a/packages/router_js/lib/router.ts +++ b/packages/router_js/lib/router.ts @@ -208,19 +208,27 @@ export default abstract class Router { if (routeInfosEqual(newState.routeInfos, oldState!.routeInfos)) { // This is a no-op transition. See if query params changed. if (queryParamChangelist) { - let newTransition = this.queryParamsTransition( - queryParamChangelist, - wasTransitioning, - oldState!, - newState - ); - newTransition.queryParamsOnly = true; - // SAFETY: The returned OpaqueTransition should actually be this. - return newTransition as InternalTransition; + if (!wasTransitioning) { + // Not in an active transition — use the QP-only fast path + // which updates the URL without re-resolving route hooks. + let newTransition = this.queryParamsTransition( + queryParamChangelist, + wasTransitioning, + oldState!, + newState + ); + newTransition.queryParamsOnly = true; + // SAFETY: The returned OpaqueTransition should actually be this. + return newTransition as InternalTransition; + } + // When a query-param-only transition is started during an active + // transition (for example, from beforeModel/afterModel), avoid the + // QP-only fast path and intentionally fall through to the normal + // transition path so it can abort and replace the active transition. + } else { + // No-op. No need to create a new transition. + return this.activeTransition || new InternalTransition(this, undefined, undefined); } - - // No-op. No need to create a new transition. - return this.activeTransition || new InternalTransition(this, undefined, undefined); } if (isIntermediate) {