Skip to content

Commit 926018d

Browse files
committed
[BUGFIX] Fix query param redirects during active transitions
1 parent e633fbd commit 926018d

2 files changed

Lines changed: 101 additions & 12 deletions

File tree

packages/ember/tests/routing/router_service_test/transitionTo_test.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,5 +439,86 @@ moduleFor(
439439
assert.equal(this.routerService.get('currentURL'), '/?url_sort=a');
440440
});
441441
}
442+
443+
async ['@test Redirecting to same route with query params from beforeModel works via transition'](
444+
assert
445+
) {
446+
assert.expect(2);
447+
448+
this.add(
449+
'route:parent.child',
450+
class extends Route {
451+
@service
452+
router;
453+
454+
beforeModel(transition) {
455+
if (!transition.to.queryParams.bar) {
456+
return this.router.transitionTo('parent.child', {
457+
queryParams: { bar: '1' },
458+
});
459+
}
460+
}
461+
}
462+
);
463+
464+
this.add(
465+
'controller:parent.child',
466+
Controller.extend({
467+
queryParams: ['bar'],
468+
bar: null,
469+
})
470+
);
471+
472+
await this.visit('/');
473+
await this.routerService.transitionTo('parent.child').followRedirects();
474+
475+
assert.equal(
476+
this.routerService.get('currentURL'),
477+
'/child?bar=1',
478+
'redirected to self with query param'
479+
);
480+
481+
assert.equal(this.routerService.get('currentRouteName'), 'parent.child');
482+
}
483+
484+
async ['@test Redirecting to same route with query params from beforeModel works via direct visit'](
485+
assert
486+
) {
487+
assert.expect(2);
488+
489+
this.add(
490+
'route:parent.child',
491+
class extends Route {
492+
@service
493+
router;
494+
495+
beforeModel(transition) {
496+
if (!transition.to.queryParams.bar) {
497+
return this.router.transitionTo('parent.child', {
498+
queryParams: { bar: '1' },
499+
});
500+
}
501+
}
502+
}
503+
);
504+
505+
this.add(
506+
'controller:parent.child',
507+
Controller.extend({
508+
queryParams: ['bar'],
509+
bar: null,
510+
})
511+
);
512+
513+
await this.visit('/child');
514+
515+
assert.equal(
516+
this.routerService.get('currentURL'),
517+
'/child?bar=1',
518+
'redirected to self with query param'
519+
);
520+
521+
assert.equal(this.routerService.get('currentRouteName'), 'parent.child');
522+
}
442523
}
443524
);

packages/router_js/lib/router.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -208,19 +208,27 @@ export default abstract class Router<R extends Route> {
208208
if (routeInfosEqual(newState.routeInfos, oldState!.routeInfos)) {
209209
// This is a no-op transition. See if query params changed.
210210
if (queryParamChangelist) {
211-
let newTransition = this.queryParamsTransition(
212-
queryParamChangelist,
213-
wasTransitioning,
214-
oldState!,
215-
newState
216-
);
217-
newTransition.queryParamsOnly = true;
218-
// SAFETY: The returned OpaqueTransition should actually be this.
219-
return newTransition as InternalTransition<R>;
211+
if (!wasTransitioning) {
212+
// Not in an active transition — use the QP-only fast path
213+
// which updates the URL without re-resolving route hooks.
214+
let newTransition = this.queryParamsTransition(
215+
queryParamChangelist,
216+
wasTransitioning,
217+
oldState!,
218+
newState
219+
);
220+
newTransition.queryParamsOnly = true;
221+
// SAFETY: The returned OpaqueTransition should actually be this.
222+
return newTransition as InternalTransition<R>;
223+
}
224+
// When a query-param-only transition is started during an active
225+
// transition (for example, from beforeModel/afterModel), avoid the
226+
// QP-only fast path and intentionally fall through to the normal
227+
// transition path so it can abort and replace the active transition.
228+
} else {
229+
// No-op. No need to create a new transition.
230+
return this.activeTransition || new InternalTransition(this, undefined, undefined);
220231
}
221-
222-
// No-op. No need to create a new transition.
223-
return this.activeTransition || new InternalTransition(this, undefined, undefined);
224232
}
225233

226234
if (isIntermediate) {

0 commit comments

Comments
 (0)