amiel / ember-data-url-templates

an ember-addon to allow building urls with url templates instead of defining buildURL
https://github.com/amiel/ember-data-url-templates/wiki
MIT License
129 stars 22 forks source link

Arrays not working right #22

Open mshappe opened 8 years ago

mshappe commented 8 years ago

Querying with something like:

{ foo: ["1", "2"] }

I cannot seem to construct any template that will actually give me

?foo[]=1&foo[]=2

Based on the documentation the following SHOULD work:

queryUrlTemplate: '{+host}/api/v2/whatever{?foo[]*}'

But in fact {?foo[]*} is entirely ignored.

amiel commented 8 years ago

@mshappe Sorry for the delayed response. I had a rushed deadline last week.

I agree that this is confusing. The issue is that the [] is not a part of the URI template syntax, it is just a part of the variable name. By contrast, in this example, the ? and * are a part of the URI template syntax.

The following works for me:

t = new UriTemplate('{?list[]*}')
t.fillFromObject({  "list[]": ["magenta", "cyan", "yellow"] })
// => "?list[]=magenta&list[]=cyan&list[]=yellow"

It would be nice to have first-class support for this kind of array syntax, but it is not supported by the URI template spec. For now, is there something that could be changed to make the documentation clearer?

mshappe commented 8 years ago

The problem does not appear to be in the underlying UriTemplate library. The problem appears to be in this one. If I revert to the latest version 0.1.x, then {?query[]*} at least works as expected, producing the array format above; but in version 0.2.0, it produces no query at all.

amiel commented 8 years ago

@mshappe oh, I misunderstood. I'll have to do some testing to see if I can reproduce.

Can you give me some more information that might help me reproduce this? What versions of ember.js and ember-data are you using?

Thanks

mshappe commented 8 years ago

Just moved up to Ember 2.3.x (yes, I know, way behind, but we were Ember 1.13.15 before that, so this is a huge advance :smile:). Ahead of that, I advanced all the other dependencies I could, so I actually moved this up to 0.2.0 before that jump. At first blush everything looked fine, but then I went to a page that relied on a query string to the server to produce filtered results and realized I was instead getting fully unfiltered results. I looked ember serve's stdout as well as the server-side log and realized that no query string was being sent at all. I experimented with various things that the docs say should work and sometimes got a format of foo=2,3,4 which of course got interpreted as just a string and broke the API. Nothing I did produced the right format.

Realizing that it USED to work, I just backed off to 0.1.5, and that's working fine for now.

amiel commented 8 years ago

@mshappe,

yes, I know, way behind, but we were Ember 1.13.15 before that

Yeah, we're also at 1.13.14 at work. Upgrading is on the TODO list :/

Thanks for all this information. The story of your issue is very helpful and I'm sorry I broke your application by releasing 0.2.0.

Here's my guess as to what happened in your case. With 0.1.5, your query { foo: ["1", "2"] } is not getting rendered with {?foo[]*}, but with jquery in DS.RESTAdapter.query.

0.2.0 includes this fix, which removes existing behavior where ember-data would add query params independently of ember-data-url-templates.

So, when you upgraded to 0.2.0, the jquery parsing of your query was removed.

To fix your issue in 0.2.0, you could try any of the following:

  1. Pass foo with brackets

    In other words, change your query to:

    { "foo[]": ["1", "2"] }
  2. Disable sortQueryParams

    ember-data-url-templates resolved the original issue by overwriting sortQueryParams.

    Because of this line in DS.RESTAdapter, you could set sortQueryParams to false in your adapter, and bypass the ember-data-url-templates hack.

    sortQueryParams: false,
  3. Overwrite sortQueryParams

    You could also overwrite sortQueryParams, either by copying the original implementation, or just passing through the params.

    sortQueryParams(params) {
     return params;
    },

I would love to hear if any of these work for you.

mshappe commented 8 years ago

@amiel Thank you for this--this explanation makes sense. I will not have a chance to play with this for a day or two (I'm currently elbows-deep in UI changes instead :smile:) but will try to get to it soon and let you know!

mshappe commented 8 years ago

And yes, I think the docco could be made clearer -- once I verify that your suggestions make it all behave, I may suggest edits to the Wiki. Again, thank you for your prompt and thorough response!

amiel commented 8 years ago

but will try to get to it soon and let you know!

no rush; thanks for looking in to it

I may suggest edits to the Wiki

Please do, I would appreciate suggested edits very much

amiel commented 7 years ago

FYI: I've got a little more time to work on ember-data-url-templates now and I'm planning to at least try out a feature that supports these nested structures.

sukima commented 6 years ago

Just my $0.02 but I don't think I would have expected {?foo[]*}/{ "foo": ["1", "2"] } to have ever worked. It is not part of the spec. I also think I would be more surprised if it did work then if it stopped working. I would also err on the side of the spec as the fact of it previously working only highlights a bug in the implementation which could (and did) get fixed. I would not wish to rely on a hidden bug/feature but instead stick to the specs: {?foo[]*}/{ "foo[]": ["1", "2"] }.