Error with RouterService.isActive with Query Parameters

[rtablada]

When working with the RouterService's isActive method default query params and non-included query params are not accounted for properly.
As a user the expected behavior would be that isActive matches the active state of link-to.

This can be seen in this twiddle:

https://ember-twiddle.com/bad1dd32c81f9c4d93864cd0a59e19ac/77a1ab275385a99cdf156bf90704abed2e144c72?openFiles=controllers.application.js%2C

From digging into (and trying to modify tests in the Ember source) it looks like there is a false positive in the tests because of the way that locationType: 'none' handles QP default values.

In our application I'm able to get isActive to return true if locationType is set to none, but when using hash or history the value of isActive does not work for default values or if there are other QPs which are set to default.

[rwjblue]

Would you mind being a bit more descriptive here? When you say:

default query params and non-included query params are not accounted for properly

And:

when using hash or history the value of isActive does not work for default values

It assumes some fundamentals that I’m not sure of. I’d love it if you could share what you are passing to isActive, what the current url is, and what you expected. Sorry for not gathering all this from the twiddle (it’s a bit hard to do on mobile).

[rwjblue]

For example, it is my understanding that the router service should never be eliding default query param values and therefore I would not expect isActive to return true when given a query param but that QP is not actually present in the URL.

[rtablada]

Sure thing!

Here’s some of the controller code from the twiddle:

Ember.Controller.extend({
  router: service(),
  appName: 'Ember Twiddle',
  value: 0,
  
  queryParams: ['value', 'value2'],
  
  isActive0: computed('router.currentURL', function() {
    return this.router.isActive('index', { queryParams: {
      value: 0
    } });
  }),

In this case I would expect that isActive should return true when the page loads and value is 0 based on the default (this should give the same result as the active state for link-to.

---

For the locationType config differences

In testing in our application I found that when using locationType: none isActive behaves as expected and matches the behavior of the active state for link to, but I was unable to recreate that scenario in a new application.
I can get that example application uploaded in a bit when I get to my computer

[rtablada]

I've updated the section about locationType: none from my last comment, it seems to be flakey. In our application I was able to have isActive match the isActive calculations of link-to but was unable to recreate it in a new app this morning (I thought I had recreated it yesterday but was misremembering).

Here is a new application running showing the unexpected behavior of isActive: http://link-to-example.surge.sh

In each case the left is a SimpleLink component which uses the router service and on the left is a matching link-to. The expected behavior would be that the active states match for both sides. link-to is behaving as expected and only comparing provided values and accounts for default values which are hidden in the current url, while isActive returns a different result.

Here is the code for easier reading on mobile/email:

Simple Link Component

import Component from '@ember/component';
import { computed } from '@ember/object';
import { inject as service } from '@ember/service';

export default Component.extend({
  tagName: '',
  router: service(),

  url: computed('routeName', 'queryParams', 'router.currentURL', function() {
    return this.router.urlFor(this.routeName, { queryParams: Object.assign({}, this.queryParams)});
  }),

  isActive: computed('routeName', 'queryParams', 'router.currentURL', function() {
    return this.router.isActive(this.routeName, { queryParams: Object.assign({}, this.queryParams)});
  }),

  actions: {
    navigate(ev) {
      ev.preventDefault();

      return this.router.transitionTo(this.routeName, { queryParams: Object.assign({}, this.queryParams)});
    }
  }
});

Simple Link Template

<a
  href={{url}}
  class="simple-link {{if isActive "active"}}"
  onclick={{action "navigate"}}
>
  {{yield}}
</a>

Index Template

<div class="flex">
  <div class="child">
    <h2>SimpleLink (Router.isActive)</h2>
    <SimpleLink @routeName="index" @queryParams={{hash sort="quantity"}}>Sort=Quantity no Filter</SimpleLink>
    <SimpleLink @routeName="index" @queryParams={{hash sort="date"}}>Sort=Date no Filter</SimpleLink>
    <SimpleLink
      @routeName="index"
      @queryParams={{hash sort="quantity" filter="value:gte=3"}}
    >
      Sort=Quantity Filter=value:gte=3
    </SimpleLink>
    <SimpleLink
      @routeName="index"
      @queryParams={{hash sort="date" filter="value:gte=3"}}
    >
      Sort=Date Filter=value:gte=3
    </SimpleLink>
    <SimpleLink
      @routeName="index"
      @queryParams={{hash sort="date" filter="somethingelse"}}
    >
      Sort=Date Filter=somethingelse
    </SimpleLink>
  </div>

  <div class="child">
    <h2>Link To</h2>
    {{#link-to "index" (query-params sort="quantity") class="btn"}}Sort=Quantity no Filter{{/link-to}}
    {{#link-to "index" (query-params sort="date") class="btn"}}Sort=Date no Filter{{/link-to}}
    {{#link-to "index" (query-params sort="quantity" filter="value:gte=3") class="btn"}}
      Sort=Quantity Filter=value:gte=3
    {{/link-to}}
    {{#link-to "index" (query-params sort="date" filter="value:gte=3") class="btn"}}
      Sort=Date Filter=value:gte=3
    {{/link-to}}
    {{#link-to "index" (query-params sort="date" filter="somethingelse") class="btn"}}
      Sort=Date Filter=somethingelse
    {{/link-to}}
  </div>
</div>

Index Controller

import Controller from '@ember/controller';
import { inject as service } from '@ember/service';

export default Controller.extend({
  router: service(),
  sort: 'quantity',
  filter: 'value:gte=3',
  queryParams: ['sort', 'filter'],
});

The full repo is here: https://github.com/rtablada/link-to-isactive-example

I'm working to add tests and I have some new ideas as to what may be causing the tests in the Ember source to pass when behavior is erratic in actual applications.

[rtablada]

I have added tests and Travis CI integration: https://travis-ci.org/rtablada/link-to-isactive-example/builds/475314915

[ryanto]

Hey @rtablada I just hit this one today. That reproduction repo is awesome btw!

I'm about to hack a fix into my app, but just wanted to see if you came up with any clever solutions to get isActive + query params working?

[validkeys]

@ryanto Do you happen to remember what your hack was to get this working as expected?

[validkeys]

The issue appears to be how the router service's "isActive" method compares query parameters. If I pass in:

router.isActive("my.route", 1, {queryParams:{filter:"done"}}

The router service's isActive method first calls:

     this._router._prepareQueryParams(
        routeName as string,
        models,
        queryParams as QueryParam,
        true /* fromRouterService */
      );

Which finds the default queryParameters for the route and adds them to the queryParams object that I passed into the isActive method. The issue here is that it then compares this "hydrated" queryParams object that I passed in against routerMicrolib.state!.queryParams (which does not contain the default queryParameters and therefore seems to always fail).

So as a hacky solution, I made my own "isActive" method which essentially works in two phases:

1. Pass the route and models to the router service's "isActive" method
2. Grab the query parameters currently acknowledged by the router and then remove any keys in my current queryParams object that do not exist in the router's query params object. And then I compare the two

I know it's hacky and for sure will have some problems in the future, but at least the solution exposed the underlying problem for me:

  @alias('router._router.currentState.routerJsState.fullQueryParams')
  stateParams

  // given the @query argument, removes any keys from that
  // object not contained in the router's state. The issue here is that
  // the router service's "isActive" method fucks up when using query parameters
  // because the router knows the default query parameters for any given route
  // and removes them from the comparison when using the link-to component
  // but does not remove them when using the router service directly
  // SEE: https://github.com/emberjs/ember.js/issues/17423
  @computed('query', 'route', 'currentURL', 'stateParams')
  get cleanedQueryParams() {
    const { query, stateParams } = this

    if (!query) {
      return query
    }

    const stateQps = Object.keys(stateParams || {})
    const cleanedParams = pick(query, stateQps)

    return cleanedParams
  }

  @computed('baseParams', 'cleanedQueryParams', 'currentURL', 'stateParams')
  get isActive() {
    const {
      router,
      cleanedQueryParams: queryParams,
      stateParams,
      baseParams
    } = this
    const params = [...baseParams]
    const response = router.isActive(...params)

    let queryResponse = true

    if (queryParams) {
      const stateKeys = Object.keys(stateParams)
      queryResponse = stateKeys.reduce((pass, key) => {
        if (!pass) {
          return pass
        }

        return isEqual(queryParams[key], stateParams[key])
      }, true)
    }

    return response && queryResponse
  }

[JeffreyToyotaConnected]

Has there been any progress on this bug? I'm experiencing the same issue when trying to use isActive with query params.