Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transitionTo with queryParams fails for renamed params #11977

Closed
ismyrnow opened this issue Aug 4, 2015 · 10 comments
Closed

transitionTo with queryParams fails for renamed params #11977

ismyrnow opened this issue Aug 4, 2015 · 10 comments

Comments

@ismyrnow
Copy link

ismyrnow commented Aug 4, 2015

When calling transitionTo, and passing queryParams, if one of the queryParams is a renamed in the destination controller (e.g. queryParams: [{ 'isFooterVisible': 'footer' }]), the application will fail to load. No errors are witnessed.

I have a fiddle showing the behavior here: http://ember-twiddle.com/b2a403268d6e26030630

The fiddle is setup to work by default, but includes instructions for triggering the issue. Just change the queryParam property name in test/controller.js from footer to isFooterVisible.

If the renamed controller property names are provided in place of the url query parameter names in the call to transitionTo, then it works properly (e.g. transitionTo({ queryParams: { isFooterVisible: true } }) instead of transitionTo({ queryParams: { footer: true } })). Perhaps this is the intended behavior?

This behavior is seen in Ember 1.13.6.

@ismyrnow
Copy link
Author

ismyrnow commented Aug 4, 2015

My use case is that I'm trying to perform a conditional redirect when the app loads, and persist the query params. I'm grabbing the query params from the transition argument and trying to pass them along.

My current workaround is to rename some of the query params to their modified controller property names. For example:

beforeModel: function (transition) {
  if (transition.queryParams.footer) {
    transition.queryParams.isFooterVisible = transition.queryParams.footer;
    delete transition.queryParams.footer;
  }

  this.transitionTo('test', { queryParams: transition.queryParams });
}

@pixelhandler
Copy link
Contributor

pixelhandler commented Aug 7, 2015

@ismyrnow I don't think this is a bug. Instead try a variation on your strategy to solve your use case, see: http://ember-twiddle.com/0ec50ce44f02e8075c33

for the test/controller I just added a computed property from the query param value on the controller:

import Ember from 'ember';

export default Ember.Controller.extend({
  queryParams: [
    'sidebar',
    'footer'
  ],
  sidebar: false,
  footer: false,
  isFooterVisible: Ember.computed.alias('footer')
});

Instead of swapping the queryParams during transition transition.queryParams.isFooterVisible = transition.queryParams.footer; just use the computed property isFooterVisible: Ember.computed.alias('footer')

@timmorey
Copy link

I ran into a very similar issue using ember 1.13.11. We support a query parameter called zoomTo on one of our routes, but I wanted to alias it as shouldZoomToAssignment in the controller, so I added

queryParams: {
    shouldZoomToAssignment: "zoomTo"
},
shouldZoomToAssignment: true

to the controller.

I expected this alias only applied to the controller, and that I could still use zoomTo when performing a transitionTo/transitionToRoute. However, when I called

this.transitionToRoute('projects.show.dispatch.assignments.details', assignment, { queryParams: { zoomTo: false } });

I caught an exception with this message

"Assertion Failed: You're not allowed to have more than one controller property map to the same query param key, but both `projects.show.dispatch.assignments.details:shouldZoomToAssignment` and `projects.show.dispatch.assignments.details:shouldZoomToAssignment` map to `zoomTo`. You can fix this by mapping one of the controller properties to a different query param key via the `as` config option, e.g. `shouldZoomToAssignment: { as: 'other-shouldZoomToAssignment' }`"

I'm willing to accept that the behavior is correct and my expectation was wrong, but that error message is nothing but confusing and misleading. It is claiming that the controller has two properties with the same name, which isn't really possible.

@ronco
Copy link
Contributor

ronco commented Apr 13, 2016

I'm also seeing this error with Ember 2.4.4:

Assertion Failed: You're not allowed to have more than one controller property map to the same query param key, but both `application:indexKey` and `application:indexKey` map to `index_key`. You can fix this by mapping one of the controller properties to a different query param key via the `as` config option, e.g. `indexKey: { as: 'other-indexKey' }`

In my case I'm not even passing query params to the call to replaceWith, it's blowing up in _serializeQueryParams with the query params gathered from _hydrateUnsuppliedQueryParams.

The query param works well elsewhere, not sure if it's related tot he fact that the replaceWith call is occuring during an afterModel hook.

Here is the query param definition in controllers/application.js:

  queryParams: {
    indexKey: 'index_key'
  },

@rwjblue
Copy link
Member

rwjblue commented Apr 13, 2016

I believe that @raytiley's fix from #13273 will address this also.

@pixelhandler
Copy link
Contributor

@ismyrnow this should be fixed now, closing.

@pixelhandler
Copy link
Contributor

oops, Reopening, confused this with another recent fix.

@ismyrnow here is an update https://ember-twiddle.com/0ec50ce44f02e8075c33?openFiles=test.controller.js%2C&route=%2Ftest%3Ffooter%3Dtrue%26sidebar%3Dtrue on the repro of the issue. In the console the error is "Assertion Failed: You're not allowed to have more than one controller property map to the same query param key, but both test:isFooterVisible and test:isFooterVisible map to footer. You can fix this by mapping one of the controller properties to a different query param key via the as config option, e.g. isFooterVisible: { as: 'other-isFooterVisible' }"

@pixelhandler
Copy link
Contributor

@ismyrnow The workaround I suggested still works:

Instead of swapping the queryParams during transition transition.queryParams.isFooterVisible = transition.queryParams.footer; just use the computed property isFooterVisible: Ember.computed.alias('footer')

If the workaround is acceptable in your use case of using a transition perhaps closet his out.

@raytiley
Copy link
Contributor

@ismyrnow Could you retest this against latest? This just got merged in: #13273

/CC @rwjblue

@rwjblue
Copy link
Member

rwjblue commented Jun 27, 2016

Thanks @raytiley. I'm going to close this for now, but I would love to land a new query params test to ensure we do not regress here in the future. Feel free to ping me if you would like help on how that might look...

@rwjblue rwjblue closed this as completed Jun 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants