Skip to content

Commit

Permalink
[BUGFIX RELEASE] Do not refresh routes on initial transition
Browse files Browse the repository at this point in the history
Refreshing the route heighercy do to query param change on the initial transition is pointless, so now we only call refresh if there is a `currentState`. This fixes #10945

(cherry picked from commit 92cd0a2)
  • Loading branch information
raytiley authored and rwjblue committed Apr 3, 2016
1 parent ac82c8d commit cbbfe4a
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/ember-routing/lib/system/route.js
Original file line number Diff line number Diff line change
Expand Up @@ -759,7 +759,7 @@ var Route = EmberObject.extend(ActionHandler, Evented, {
var totalChanged = Object.keys(changed).concat(Object.keys(removed));
for (var i = 0, len = totalChanged.length; i < len; ++i) {
var qp = qpMap[totalChanged[i]];
if (qp && get(this._optionsForQueryParam(qp), 'refreshModel')) {
if (qp && get(this._optionsForQueryParam(qp), 'refreshModel') && this.router.currentState) {
this.refresh();
}
}
Expand Down
54 changes: 54 additions & 0 deletions packages/ember/tests/routing/query_params_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,33 @@ if (isEnabled('ember-routing-route-configured-query-params')) {
equal(indexModelCount, 2);
});

QUnit.test('refreshModel does not cause a second transition during app boot ', function() {
expect(0);

App.ApplicationRoute = Route.extend({
queryParams: {
appomg: {
defaultValue: 'applol'
}
}
});

App.IndexRoute = Route.extend({
queryParams: {
omg: {
defaultValue: 'lol',
refreshModel: true
}
},
refresh: function() {
ok(false);
}
});

startingURL = '/?appomg=hello&omg=world';
bootApplication();
});

QUnit.test('can use refreshModel even w URL changes that remove QPs from address bar when QP configured on route', function() {
expect(4);

Expand Down Expand Up @@ -2435,6 +2462,33 @@ if (isEnabled('ember-routing-route-configured-query-params')) {
equal(indexModelCount, 2);
});

QUnit.test('refreshModel does not cause a second transition during app boot ', function() {
expect(0);
App.ApplicationController = Controller.extend({
queryParams: ['appomg'],
appomg: 'applol'
});

App.IndexController = Controller.extend({
queryParams: ['omg'],
omg: 'lol'
});

App.IndexRoute = Route.extend({
queryParams: {
omg: {
refreshModel: true
}
},
refresh: function() {
ok(false);
}
});

startingURL = '/?appomg=hello&omg=world';
bootApplication();
});

QUnit.test('Use Ember.get to retrieve query params \'refreshModel\' configuration', function() {
expect(6);
App.ApplicationController = Controller.extend({
Expand Down

3 comments on commit cbbfe4a

@wagenet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also not call refresh if the route isn't active? I'm seeing situations where queryParamsDidChange gets called on a route that isn't the active route yet.

@rwjblue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wagenet - Can you open an issue and try to work up a ember-twiddle.com demo?

@wagenet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.