diff --git a/packages/@ember/-internals/routing/lib/services/router.ts b/packages/@ember/-internals/routing/lib/services/router.ts index 276143f2e57..6a5e2450af8 100644 --- a/packages/@ember/-internals/routing/lib/services/router.ts +++ b/packages/@ember/-internals/routing/lib/services/router.ts @@ -133,7 +133,6 @@ class RouterService extends Service.extend(Evented) { let { routeName, models, queryParams } = extractRouteArgs(args); let transition = this._router._doTransition(routeName, models, queryParams, true); - transition['_keepDefaultQueryParamValues'] = true; return transition; } @@ -324,21 +323,31 @@ class RouterService extends Service.extend(Evented) { let hasQueryParams = Object.keys(queryParams).length > 0; if (hasQueryParams) { + // UNSAFE: casting `routeName as string` here encodes the existing + // assumption but may be wrong: `extractRouteArgs` correctly returns it + // as `string | undefined`. There may be bugs if `_prepareQueryParams` + // does not correctly account for `undefined` values for `routeName`. + // Spoilers: under the hood this currently uses router.js APIs which + // *do not* account for this being `undefined`. + let targetRouteName = routeName as string; + queryParams = Object.assign({}, queryParams); this._router._prepareQueryParams( - // UNSAFE: casting `routeName as string` here encodes the existing - // assumption but may be wrong: `extractRouteArgs` correctly returns it - // as `string | undefined`. There may be bugs if `_prepareQueryParams` - // does not correctly account for `undefined` values for `routeName`. - // Spoilers: under the hood this currently uses router.js APIs which - // *do not* account for this being `undefined`. - routeName as string, + targetRouteName, models, queryParams, true /* fromRouterService */ ); - return shallowEqual(queryParams, routerMicrolib.state!.queryParams); + let currentQueryParams = Object.assign({}, routerMicrolib.state!.queryParams); + this._router._prepareQueryParams( + targetRouteName, + models, + currentQueryParams, + true /* fromRouterService */ + ); + + return shallowEqual(queryParams, currentQueryParams); } return true; diff --git a/packages/@ember/-internals/routing/lib/system/route.ts b/packages/@ember/-internals/routing/lib/system/route.ts index 1e2586d8ed3..e239ee2da3f 100644 --- a/packages/@ember/-internals/routing/lib/system/route.ts +++ b/packages/@ember/-internals/routing/lib/system/route.ts @@ -2522,7 +2522,7 @@ Route.reopen({ qp.serializedValue = svalue; let thisQueryParamHasDefaultValue = qp.serializedDefaultValue === svalue; - if (!thisQueryParamHasDefaultValue || (transition as any)._keepDefaultQueryParamValues) { + if (!thisQueryParamHasDefaultValue) { finalParams.push({ value: svalue, visible: true, diff --git a/packages/@ember/-internals/routing/lib/system/router.ts b/packages/@ember/-internals/routing/lib/system/router.ts index 39d40c624b0..1b3ce694b0a 100644 --- a/packages/@ember/-internals/routing/lib/system/router.ts +++ b/packages/@ember/-internals/routing/lib/system/router.ts @@ -1053,7 +1053,7 @@ class EmberRouter extends EmberObject.extend(Evented) i _targetRouteName: string | undefined, models: ModelFor[], _queryParams: Record, - _keepDefaultQueryParamValues?: boolean + _fromRouterService?: boolean ): Transition { let targetRouteName = _targetRouteName || getActiveTargetName(this._routerMicrolib); assert( @@ -1069,12 +1069,7 @@ class EmberRouter extends EmberObject.extend(Evented) i Object.assign(queryParams, _queryParams); - this._prepareQueryParams( - targetRouteName, - models, - queryParams, - Boolean(_keepDefaultQueryParamValues) - ); + this._prepareQueryParams(targetRouteName, models, queryParams, Boolean(_fromRouterService)); let transition = this._routerMicrolib.transitionTo(targetRouteName, ...models, { queryParams }); diff --git a/packages/ember/tests/routing/router_service_test/isActive_test.js b/packages/ember/tests/routing/router_service_test/isActive_test.js index 69f35527e8a..698124d7550 100644 --- a/packages/ember/tests/routing/router_service_test/isActive_test.js +++ b/packages/ember/tests/routing/router_service_test/isActive_test.js @@ -117,9 +117,10 @@ moduleFor( return this.routerService.transitionTo('parent.child', queryParams); }) .then(() => { - assert.ok(this.routerService.isActive('parent.child', queryParams)); + assert.ok(this.routerService.isActive('parent.child', queryParams), 'route is active'); assert.notOk( - this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'DESC' })) + this.routerService.isActive('parent.child', this.buildQueryParams({ sort: 'DESC' })), + 'route with QPs is not active' ); }); } diff --git a/packages/ember/tests/routing/router_service_test/replaceWith_test.js b/packages/ember/tests/routing/router_service_test/replaceWith_test.js index a1ca0bca866..ea0683b13a3 100644 --- a/packages/ember/tests/routing/router_service_test/replaceWith_test.js +++ b/packages/ember/tests/routing/router_service_test/replaceWith_test.js @@ -107,7 +107,7 @@ moduleFor( }); } - ['@test RouterService#replaceWith with basic query params does not remove query param defaults']( + ['@test RouterService#replaceWith with basic query params removes query param defaults']( assert ) { assert.expect(1); @@ -133,7 +133,7 @@ moduleFor( return this.routerService.replaceWith('parent.child', queryParams); }) .then(() => { - assert.deepEqual(this.state, ['/', '/child?sort=ASC']); + assert.deepEqual(this.state, ['/', '/child']); }); } } 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 fc94312b6af..a5c84311586 100644 --- a/packages/ember/tests/routing/router_service_test/transitionTo_test.js +++ b/packages/ember/tests/routing/router_service_test/transitionTo_test.js @@ -238,7 +238,7 @@ moduleFor( this.assertText('much dynamicism'); } - ['@test RouterService#transitionTo with basic query params does not remove query param defaults']( + ['@test RouterService#transitionTo with basic query params removes query param defaults']( assert ) { assert.expect(1); @@ -258,7 +258,7 @@ moduleFor( return this.routerService.transitionTo('parent.child', queryParams); }) .then(() => { - assert.equal(this.routerService.get('currentURL'), '/child?sort=ASC'); + assert.equal(this.routerService.get('currentURL'), '/child'); }); } @@ -302,14 +302,14 @@ moduleFor( }) ); - let queryParams = this.buildQueryParams({ sort: 'ASC' }); + let queryParams = this.buildQueryParams({ sort: 'DESC' }); return this.visit('/') .then(() => { return this.routerService.transitionTo('parent.child', queryParams); }) .then(() => { - assert.equal(this.routerService.get('currentURL'), '/child?sort=ASC'); + assert.equal(this.routerService.get('currentURL'), '/child?sort=DESC'); }); } @@ -328,14 +328,14 @@ moduleFor( }) ); - let queryParams = this.buildQueryParams({ url_sort: 'ASC' }); + let queryParams = this.buildQueryParams({ url_sort: 'DESC' }); return this.visit('/') .then(() => { return this.routerService.transitionTo('parent.child', queryParams); }) .then(() => { - assert.equal(this.routerService.get('currentURL'), '/child?url_sort=ASC'); + assert.equal(this.routerService.get('currentURL'), '/child?url_sort=DESC'); }); }