Skip to content

buildQuery RFC - #135

Closed
grapho wants to merge 6 commits into
emberjs:masterfrom
grapho:build-query-rfc
Closed

buildQuery RFC#135
grapho wants to merge 6 commits into
emberjs:masterfrom
grapho:build-query-rfc

Conversation

@grapho

@grapho grapho commented Apr 15, 2016

Copy link
Copy Markdown

This is my first ever RFC. I hope it is good!

Let me know what you think!

https://github.com/grapho/rfcs/blob/build-query-rfc/new-build-query-adapter-api.md

@mmun mmun added the T-ember-data RFCs that impact the ember-data library label Apr 15, 2016
@grapho grapho changed the title Build query rfc buildQuery RFC Apr 15, 2016
@cibernox

Copy link
Copy Markdown
Contributor

@grapho For normalizing queryParams I resorted many times to override the query and queryRecord hooks in the adapter, where I mangle the query object, rename keys and then call this._super.

Can you elaborate why you think this approach is not enough?

@grapho

grapho commented May 24, 2016

Copy link
Copy Markdown
Author

@cibernox my goal is to make the process just a little simpler, even provide a dedicated place to perform said "mangling" :P

In my project environment, all of our APIs use snake_case keys even for query params, so I have spent a lot of time camelCasing my keys to make things "prettier" for ember. I have done the same thing you described with many of my query and queryRecords, until I decided to dry up my code and make a couple custom methods.

I figured maybe more people might find this PR convenient if they find themselves in similar use case

@cibernox

Copy link
Copy Markdown
Contributor

@grapho What about overriding queryRecord and query only once in the ApplicationSerializer which usually all other serializers extend from?

function camelCaseObject(obj) {
  // camelCase here
}
export default Serializer.extend({
  queryRecord(store, type, query) {
    camelCaseObject(query);
    return this._super(...arguments);
  },

  query(store, type, query) {
    camelCaseObject(query);
    return this._super(...arguments);
  }
});

Given that this doesn't enable any new feature, I'm just trying to balance if this significantly more convenient that what we have right now so it existence is justified. I don't know how the core team feels about increasing the API surface of the adapters.

Even if this goes ahead, I think that the naming is a bit confusing, because buildQuery to me sounds like if this method is going to build the actual queryString. I think that normalizeQuery would be less open to interpretation.

@grapho

grapho commented May 24, 2016

Copy link
Copy Markdown
Author

Good points, @cibernox. I'll leave this open and see if any more opinions pop up.. if no one else seems interested, I can close.

@grapho grapho closed this Nov 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-ember-data RFCs that impact the ember-data library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants