[Bug] Router service transitionTo adds unspecified query params (with their default values)

[pgengler]

🐞 Describe the Bug

When using the transitionTo and replaceWith methods on the RouterService, query params that are specified on the controller for the new route, and which have default values with are non-null, are added to the resulting URL, even though they are not specified in the transitionTo or replaceWith call.

🔬 Minimal Reproduction

A small reproduction (with a failing test) is here: https://ember-twiddle.com/a796bad00bded99fa45bfbaab159b485?openFiles=tests.acceptance.transition-to-query-params-test%5C.js

😕 Actual Behavior

Using RouterService#transitionTo adds query parameters that have non-null default values on the controller but were not specified in the transitionTo call.

🤔 Expected Behavior

Using RouterService#transitionTo should not add unspecified query parameters to the URL (it should match the (now-deprecated) Controller#transitionToRoute and Route#transitionTo methods.)

🌍 Environment

• Ember: - from at least 3.11.1 (earliest version the ember-twiddle example runs with) through at least 3.26.0

➕ Additional Context

I know there have been a few issues about RouterService#transitionTo not pruning query parameters that have default values (e.g., #19492), but this is different in that it is added query parameters, with their default values. That makes sense given the Router Service RFC's note that "default values will not be stripped from generated URLs."

I think the behavior described here is a bug it's adding query params that weren't specified, and it's behavior which contravenes the RFC's rationale for not pruning default-valued query params.
The RFC notes that

Determining their default values is expensive, because it involves instantiating the corresponding controller, even in cases where we will never visit its route.

However, in this case, in order to 1) discover the query params, and 2) discover their default values in order to add them to the URL, the controller had to be instantiated anyway.

[pgengler]

Just to clarify, what I mean is that the end state (at the end of the transition, not any intermediate state) includes the query params with default values added to the URL.

E.g., a plain call like this.router.transitionTo('b') (no query params specified on the transitionTo call) settles with a URL that includes query params (e.g., /b?bar=&baz=not%20null)

[johanrd]

Yes, this came as a surprise to me as well. When moving from the deprecated this.replaceWith to this.router.replaceWith the URL now gets clogged with a lot of ugly default queryParams.

Any clean way to work around this?

[rahulk94]

Just ran into this as well.

The only way I see around this (which may help someone) to call this.router.transitionTo with a pre-made URL rather than passing a model and a query params object.

    const url = this.router.urlFor(
      'routeName',
      {
         someModel: 'hello'
      },
      {
        queryParams: { qpOne: 'valueOne', qpTwo: 'valueTwo' }
      }
    );
    this.router.transitionTo(url);

However for my case at least, this is still problematic due to some other issues (potentially bugs).

[johanrd]

Update: This does not seem to be an issue anymore for me when running ember-source: 3.28.4. Possibly related to this fix: #19733

[nag5000]

Update: This does not seem to be an issue anymore for me when running ember-source: 3.28.4.

Just tried, it still reproduces on clean ember application with ember-source: 3.28.4.

[wagenet]

Whoa, the issue is just this line:

ember.js/packages/@ember/-internals/routing/lib/services/router.ts

Line 136 in 8455848

transition['_keepDefaultQueryParamValues'] = true;

[dwickern]

Here's a workaround if you're on Ember < 4.4

// app/services/router.js
import { default as EmberRouterService } from '@ember/-internals/routing/lib/services/router';

export default class RouterService extends EmberRouterService {
  transitionTo(...args) {
    const transition = super.transitionTo(...args);
    transition._keepDefaultQueryParamValues = false;
    return transition;
  }
}

[wagenet]

@dwickern what version are you on? Do we need to backport?

[dwickern]

@dwickern what version are you on? Do we need to backport?

I'm on 3.28.9, trying to upgrade. I don't think a backport is needed since routing.transition-methods are supported until version 5. It's not a blocker for me.

[RobbieTheWagner]

@wagenet too late for a backport to 3.28?

[RobbieTheWagner]

@wagenet we would still love a backport to 3.28 here.

[kategengler]

@RobbieTheWagner 3.28 hasn't been supported for bug fixes since May 2022, and security fixes since September 2022. I'm not going to say never, but we're unlikely to backport this, at this point, especially since it affected back to 3.11 and so most projects will have compensated by now.