ember-latest with query-params feature breaks ember-auth

[flynfish]

Just wanted to give a heads up on this since it will be turned on by default with ember 1.1 release. Turning on the query-params feature causes ember-auth to break and the user is no longer transitioned to the logged in route. (A page refresh sends them to the correct route though)

Ember.FEATURES["query-params"] = true;

 model.on('didCreateRecord', function() {
        var user = this._data;
        var auth = {auth_token: user.auth_token, user_id: user.id, remember_token: user.remember_token};
        App.Auth.createSession(auth);
        App.Auth.get("user").reload();
      });

About to start trying to debug to figure out why, any ideas on where to start would be appreciated!

[heartsentwined]

I'm actually more interested in knowing the expected public API for accessing query params, and use that over the existing hack. It's really an ugly hack, basically hijacking the router initialization process to grab the query string params before ember trims / cleans them away.

[flynfish]

@heartsentwined when you mention existing hack, are you talking about something ember-auth is doing or the way the ember-query stuff works?

[flynfish]

Debugging:

App.Auth.createSession(auth); ➡️

create: function (input) {
    get$(get$(this, 'auth'), '_response').canonicalize(input);
    return get$(this, 'auth').trigger('signInSuccess');
  },

➡️

get$(this, 'auth').on('signInSuccess', (this$ = this, function () {
      return this$.start();
    }));

➡️

start: function () {
    set$(this, 'signedIn', true);
    set$(this, 'startTime', new Date);
    set$(this, 'endTime', null);
    return this.findUser();
  },

➡️

enters set$(this, 'signedIn', true);

➡️

fails on result.retry()

[flynfish]

Looks like the issue is in resolveRedirect() returning the empty object:

this.get('' + env + 'Redir')

and this is caused by the resolveRedirect getting hit twice?

@heartsentwined any thoughts? We could check for empty object in resolveRedirect and use fallback if its empty.

return $.isEmptyObject(this.get('' + env + 'Redir')) ? fallback : this.get('' + env + 'Redir');

[flynfish]

So checking for empty object fixes the issue when you don't have any queryParams...but when you do the signInRedir object is populated with the queryParam (i.e localhost:3000/signup?promoCode=blah --> {promoCode: "blah"})

via 3182, queryParams can be cleared on transition:

It's also possible to clear all query params by passing false, e.g. transitionTo({queryParams: false}) or {{#linkTo 'posts' queryParams=false}}

So maybe the ember-auth signIn route would have a config option to clear queryParams if needed? By default it would keep them and just transition to the normal route with them.

[flynfish]

I think the cause is in the router, returning either [transition] or the [queryParams, transition] ->

if (handlerInfo.queryParams) {
          args = [handlerInfo.queryParams, transition];
        } else {
          args = [transition];
        }

So the ember-auth ActionRedirectable beforeModel patch needs to handle the case of queryParams being present. Currently it only receives the first arg ->

beforeModel: function (transition) {
          debugger;
          return get$(self, 'auth').followPromise(this._super.apply(this, arguments), function () {
            self.registerRedirect(transition);
            return null;
          });
        }

[flynfish]

I temporarily fixed it by checking the length of the arguments. This isn't a final solution since it doesn't support passing the queryParams on to the registerRedirect. Waiting to hear from alexspeller on the best way to handle checking for queryParams.

beforeModel: function () {
          var args = arguments;
          return get$(self, 'auth').followPromise(this._super.apply(this, arguments), function () {
            if(args.length === 1 && args[0].hasOwnProperty("sequence")){
              self.registerRedirect(args[0]);
            } else if(args.length === 2 ) {
              self.registerRedirect(args[1]);
            }

            return null;
          });
        }

[heartsentwined]

Not pretty, but it's a working hack nonetheless?

[flynfish]

@heartsentwined Yea it's working for me since I don't need the query params to continue to be available after the user is signed in.

[heartsentwined]

I think the js idiom is to go with an instanceof check, but then I discovered that the Transition object is private. https://github.com/emberjs/ember.js/blob/2fa8777a3f2830fd3aaba297d8c5a8e8d6853a82/packages/ember-routing/lib/vendor/router.js#L38

I'll fallback on a ruby-style respond_to? check on the retry method, and just assume that such a method would only be present on a real transition. Probably something like

beforeModel: (queryParams, transition) ->
  if typeof queryParams.retry == 'function' # 'undefined' if not present
    transition = queryParams

or a simpler, but less robust, check similar to your arguments.length:

beforeModel: (queryParams, transition) ->
  unless transition? # coffeescript idiom for checking null + undefined
    transition = queryParams

[heartsentwined]

The reason why I'm checking on the transition arg instead of the queryParams arg is that the former could be, essentially, just a plain js object but still functional. Example: someone stubbing queryParams to be a plain js object in a test suite.

[flynfish]

Yea I had noticed that Transition was private too, which is why I had to go the route of checking the args length. Your solution is cleaner

[heartsentwined]

@flynfish query-params support integrated to 9.x. The usual code-generating docs and upgrade guide to help with the transition.

Please reopen this ticket if it doesn't work.

[pedrokost]

I am trying the Query parameters as described in the ember guides. The moment I add the beforeModel hook it fails.

beforeModel: ->
    @_super.apply(this, arguments)

The error is thrown in the following method form the ember source:

  _setupArrangedContent: function() {
    var arrangedContent = get(this, 'arrangedContent');

    if (arrangedContent) {
      console.log(arrangedContent, Ember.isArray( arrangedContent))
      Ember.assert(Ember.String.fmt('ArrayProxy expects an Array or ' + 
        'Ember.ArrayProxy, but you passed %@', [typeof arrangedContent]), 
        Ember.isArray(arrangedContent));

      arrangedContent.addArrayObserver(this, {
        willChange: 'arrangedContentArrayWillChange',
        didChange: 'arrangedContentArrayDidChange'
      });
    }
  },

In line 4 I am printing the object that it expect to be an array. The console outputs:

Object {show: "all"} false 

which is the query parameter object.

As indicated in the Ember guide for the new query parameters, I am using Ember canary.

[alexspeller]

@pedrokost this may be related to emberjs/ember.js#4436 - but doesn't look like it. Caused by the same problem. Why is an object being assigned to an array anyway?

[kevinansfield]

I've been stuck on the last version of ember that still has the query-params feature as ember-auth didn't seem compatible with query-params-new which is what the ember guides refers to.

Has anyone had success using query-params-new?

I'm assuming it's the ember-auth-module-url-authenticatable module that needs updating but I haven't had a chance to look into it yet.