Don't refresh a parent route's model when the query param doesn't change

[misterbyrne]

Test case for #17494
Plus a potential (if a bit messy) fix.

Routes that have a query param with a default value and refreshModel: true, will be refreshed each time a transition happens - but only if that transition is invoke from the router service.

This works as expected if you invoke the transition using the internal router. This is because a some of the internal router's related behaviour is explicitly bimodal - there's a boolean parameter named _fromRouterService that controls this.

The root of the problem seems to be in the _pruneDefaultQueryParamValues method. If a transition is invoked from the router service, this currently strips default query params. This means that when later compared to the current state, the query params appear unequal, resulting in a refresh.

I've added a fix which prevents the default param values from being stripped for this specific invocation - with yet another boolean param (named _pruneDefaultQueryParamValues ). Not ideal.

[sly7-7]

@misterbyrne Thank you :). I'm trying to figure out how the router works these days, and I discovered https://github.com/emberjs/rfcs/blob/master/text/0095-router-service.md#query-parameter-semantics which I think is what you describe. So I would say the behavior is intented. That beeing said I find that inconsistent behavior may be error prone.
Plus, in the current documentation: https://guides.emberjs.com/release/routing/query-params/#toc_default-values-and-deserialization this is still referring to the default qp value are not in the url.
You have to go to https://api.emberjs.com/ember/3.25/classes/RouterService/methods/transitionTo?anchor=transitionTo to see the description of the default param value behavior.

cc/ @rwjblue

[sly7-7]

I'm curious what is the this.routerService.currentURL at this point and if it would be the same as the actual url that would appear in the browser location.

[misterbyrne]

Interesting question! I just checked, and it's /?parentParam=parentParamDefaultValue.

I think this is because it's the result of this.visit. If I transition to the same URL using the transitionTo then default value is stripped out of the URL, e.g.

      await this.visit('/');
      assert.equal(this.routerService.currentURL, '/'); // passes
      await this.routerService.transitionTo('/?parentParam=parentParamDefaultValue');
      assert.equal(this.routerService.currentURL, '/'); // passes
      await this.visit('/?parentParam=parentParamDefaultValue');
      assert.equal(this.routerService.currentURL, '/'); // fails

[misterbyrne]

This seems intentional because visit ends up calling handleURL which has a comment that explains...

    // Perform a URL-based transition, but don't change
    // the URL afterward, since it already happened.

https://github.com/tildeio/router.js/blob/8a55b2e985ff4ba71b99187b3fcd9f52f752356f/lib/router/router.ts#L866

[kategengler]

As we plan to rework the router for Polaris, we're hesitant to touch existing router code (every bug we fix is one someone else relies upon).