rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Expose query param defaults

Open kellyselden opened this issue 9 years ago • 6 comments

Rendered

kellyselden avatar Jan 27 '16 14:01 kellyselden

We have this exact use case all over the place. However, it's not just limited to resetController, we often use this.transitionTo or {{link-to}} with query-params throughout the app. When query-params are sticky we have cases where we need to reset back to the default value as part of the transition.

So I suggest that we consider expanding the scope of the RFC to provide a mechanism for other routes / components to determine the default values.

We ended up making constants for these that could be exported from a single global file. Eg:

// app/constants.js
export const ROOM_QP_SORT = 'newest';
export const ROOM_QP_FILTER = 'all';
export const ROOM_QP_SEARCH = '';
// app/controllers/room.js
import { ROOM_QP_SORT, ROOM_QP_FILTER, ROOM_QP_SEARCH } from 'app/constants';

export default Ember.Controller.extend({
  queryParams: ['sort', 'filter', 'search'],

  sort: ROOM_QP_SORT,
  filter: ROOM_QP_FILTER,
  search: ROOM_QP_SEARCH
});
// app/routes/other-route.js
import { ROOM_QP_SORT, ROOM_QP_FILTER, ROOM_QP_SEARCH } from 'app/constants';

export default Ember.Route.extend({
  actions: {
    deleteThingAndGoToList() {
      this.currentModel.destroyRecord().then(() => {
        this.transitionTo('room', { 
          queryParams: {
            sort: ROOM_QP_SORT,
            filter: ROOM_QP_FILTER,
            search: ROOM_QP_SEARCH
          }
        });
      });
    }
  }
});
{{!-- x-routable/other-route.hbs }}
{{#link-to 'room' (query-params 
    sort=(c "ROOM_QP_SORT") 
    filter=(c "ROOM_QP_FILTER") 
    search=(c "ROOM_QP_SEARCH"))}}
  Show All
{{/link-to}}

workmanw avatar Jan 27 '16 15:01 workmanw

:+1: I'm onboard with this since I think, if designed properly, the API would be beneficial to asynchronous routing. Since default QP values are serialized in URLs, having some way of exposing them (ideally, separated from the controller) could enable pre-emptive loading ahead of the actual controller.

cc @nathanhammond

trentmwillis avatar Feb 06 '16 17:02 trentmwillis

Since default QP values are serialized in URLs

@trentmwillis You mean "aren't" right?

From your link: "When a controller's query param property is currently set to its default value, this value won't be serialized into the URL"

kellyselden avatar Feb 06 '16 17:02 kellyselden

@kellyselden yeah, what I really meant was more along the lines of "are taken into consideration when serializing URLs".

trentmwillis avatar Feb 06 '16 17:02 trentmwillis

So today is the day where I finally learn query params. I've made it four years without doing anything more than trivial usage. The goal @trentmwillis mentions here is basically regarding this: https://github.com/dgeb/ember-engines/issues/6#issuecomment-180823082

Right now if you don't expose query param defaults for route targets in a global manner when constructing links in engines land you end up having the defaults appear in URLs (if they're passed). Suboptimal, but not the end of the world.

Because of this RFC and related issues it might make sense to reconsider how we do that, solve the engines problem, and solve this ergonomics problem all at once.

nathanhammond avatar Feb 06 '16 18:02 nathanhammond

I believe there is some router-related spiking happening around Polaris. That may eliminate the need for this RFC.

wagenet avatar Jul 23 '22 17:07 wagenet

Thanks for spending the time writing this RFC! Given the significant number of concerns around routing, we’re going to be completely revamping the router as part of Polaris. This RFC won’t directly apply to the new router so we’re going to move this PR to FCP to Close. But don’t worry, your work here was not in vain! We will be keeping track of this RFC and others like it to make sure that the concerns it aims to address will be covered in the new router. Your work here does a significant service to us in this regard!

wagenet avatar Dec 06 '22 23:12 wagenet