replaceWith / transitionTo doesn't respect query params on beforeModel

[bgentry]

As described in #12824 (comment), I'm attempting to use replaceWith to set a default query param value that I want to be visible in the URL.

Ember Twiddle

Expected behavior:

On initial load, the page should output "Query timeframe: 24hrs" with the desired value of the timeframe query param (24hrs).

Actual result:

The redirect does not take place, and the query param is not set to any value. The page instead outputs "Query timeframe: "

If you set the timeframe query param manually, the output reflects the actual value of that param. Try changing the path box on the Twiddle to /?timeframe=24hrs.

cc @rwjblue

[alexspeller]

This doesn't seem to be related to the recent transition work I did.

You're not really transitioning, because your query params is not refreshModel. If you really need the default param serialized to the url, I suggest using refreshModel or using set in setupController (requires a run.next) instead, both of which do work currently.

However I think the real issue is fighting against the design of query params, which is explicitly that the default value doesn't show in the url. Your use case is explicitly not support by ember currently so whilst there are some workarounds, I'm inclined to say this isn't a bug, it's just not how query params work

[bgentry]

@alexspeller Thanks for the workaround suggestions. Using run.next inside the controller init to set the param also works:

import Ember from 'ember';

export default Ember.Controller.extend({
  queryParams: 'timeframe',
  timeframe: null,

  init() {
    this._super(...arguments);

    if (this.get('timeframe') == null) {
      Ember.run.next(() => {
        this.set('timeframe', '24hrs');
      });
    }
  }
});

However, this creates a useless history item. The reason I want to have the default query param visible is that I might want to change the default but still have the old URLs work the same way. In this scenario, a history item with no query params just results in a redirect, which breaks the back button unless you hammer through it.

Both the guides and the API docs suggest that a transition during beforeModel should work. Why should replaceWith / transitionTo work differently during beforeModel than they do any other place you use them? That seems like a confusing inconsistency.

[alexspeller]

Hmm yeah, thinking about it, the transition should work, as there are totally legit usecases for doing this unrelated to param defaults.

I wouldn't advise doing pretty much anything in init methods most anywhere in ember, with the exception of basic object setup - you have no guarantees about when your init method would be called, it may not be the point you expect due to dependency injection laziness.

[pixelhandler]

@bgentry This seems like a bug seems that replaceWith and transition to do not work with a {queryParams:{name:'value'} argument; but work fine with using an argument for a route name.

• https://ember-twiddle.com/418933f4e91d2fa4b745f3423bd8ba8a?openFiles=routes.index.js%2C

Docs here, http://emberjs.com/api/classes/Ember.Route.html#method_transitionTo, indicate that the following should work, but the above that is not working in the above twiddle.

// if you just want to transition the query parameters without changing the route
this.transitionTo({ queryParams: { sort: 'date' } });

[larowlan]

Took a look at this, there is already this test:

['@test transitionTo supports query params'](assert) {
    this.setSingleQPController('index', 'foo', 'lol');

    return this.visitAndAssert('/').then(() => {
      this.transitionTo({ queryParams: { foo: 'borf' } });
      this.assertCurrentPath('/?foo=borf', 'shorthand supported');

      this.transitionTo({ queryParams: { 'index:foo': 'blaf' } });
      this.assertCurrentPath('/?foo=blaf', 'longform supported');

      this.transitionTo({ queryParams: { 'index:foo': false } });
      this.assertCurrentPath('/?foo=false', 'longform supported (bool)');

      this.transitionTo({ queryParams: { foo: false } });
      this.assertCurrentPath('/?foo=false', 'shorhand supported (bool)');
    });
  }

The first and last assert appear to be testing this behaviour already. What am I missing (new contributor so apologies if it's obvious)?

[bgentry]

@larowlan this seems to be a bug that happens specifically when this is done in the route before the controller is loaded. I don't think that the test is exercising that specific scenario.

[larowlan]

Thanks @bgentry would appreciate any tips on how to simulate that scenario in a test

[alexspeller]

@larowlan I would expect this is an issue in the router.js library, not in ember.js - I'd start there if you're looking to tackle this one

[larowlan]

thanks, think I've worked out how to test it 👍

[larowlan]

master...larowlan:14606-transitionTo-queryParams this test gives the same error as seen on the console of https://ember-twiddle.com/418933f4e91d2fa4b745f3423bd8ba8a?openFiles=routes.index.js%2C i.e.

handlerInfos[(handlerInfos.length - 1)] is undefined

Can someone give it a once over to make sure I'm on the right path?

[larowlan]

ok, got it to the point where it doesn't error, but the test fails (genuinely)

there are no handlers in the router that early...is this just not supported/possible from the beforeModel hook?

[bgentry]

is this just not supported/possible from the beforeModel hook?

The routing guides state that redirection / transitioning in beforeModel is indeed supported. It's something about the query-only case I think.

[erichiggins]

Are there any workarounds that can be implemented inside of routes? I'm trying to use beforeModel to determine if users are authenticated and if not, redirect them to login, but with query parameters to get them back to where they were -- this issue is blocking.

[alexspeller]

I think something like this would do it

beforeModel() {
  let newParams = { foo: 123 };
  (this.router.router || this.router._routerMicrolib).activeTransition.abort();
  this.replaceWith(this.routeName, {queryParams: newParams});
}

[erichiggins]

@alexspeller Here's what I tried based on your suggestion, but the query parameters are still dropped.

  beforeModel() {
    if (this.get('session.isAuthenticated') === false) {
      (this.router.router || this.router._routerMicrolib).activeTransition.abort();
      this.replaceWith('auth.login', {queryParams: {continue_to: '/checkout/address'}});
    }
  }

[alexspeller]

and you have that query param defined on your controller?

[erichiggins]

I don't use controllers at all (I don't have a use for them and I know they are being deprecated).

[alexspeller]

@erichiggins that's incorrect. You can't use query params without controllers. And they are not deprecated. In fact, you are using them, they are autogenerated for you. You should not be avoiding controllers in ember at this point.

[erichiggins]

@alexspeller Ok, thanks for the heads up. Could you point me to the documentation about query params needing to be defined in the controllers in order for redirects to pass them along? I must have missed that.

[alexspeller]

This is nothing to do with redirects. The redirect thing is probably a red herring.

Query params have to be defined on controllers to work at all. Have a look at the guides section on query params. That explains how to use then.

[erichiggins]

Adding the following to a Controller for my /auth/login route worked. Hopefully this helps others.

  queryParams: ['continue_to'],
  continue_to: null

For what it's worth, the documentation about Redirecting does not state that query parameters must be declared in the destination Controller, but it is mentioned on the Query Parameter docs. That's probably how I missed it. Making this more explicit in the docs may help other Ember newbies like myself.

[pixelhandler]

@erichiggins glad you posted the need to document with more clear info on the destination controller...

For what it's worth, the documentation about Redirecting does not state that query parameters must be declared in the destination Controller, but it is mentioned on the Query Parameter docs. That's probably how I missed it. Making this more explicit in the docs may help other Ember newbies like myself.

I'll add the label documentation as well.