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

Arrays not working right #22

Open
mshappe opened this issue Oct 12, 2016 · 10 comments
Open

Arrays not working right #22

mshappe opened this issue Oct 12, 2016 · 10 comments

Comments

@mshappe
Copy link

mshappe commented Oct 12, 2016

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
Copy link
Owner

amiel commented Oct 17, 2016

@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
Copy link
Author

mshappe commented Oct 18, 2016

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
Copy link
Owner

amiel commented Oct 18, 2016

@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
Copy link
Author

mshappe commented Oct 18, 2016

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 😄). 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
Copy link
Owner

amiel commented Oct 18, 2016

@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
Copy link
Author

mshappe commented Oct 18, 2016

@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 😄) but will try to get to it soon and let you know!

@mshappe
Copy link
Author

mshappe commented Oct 18, 2016

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
Copy link
Owner

amiel commented Oct 18, 2016

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
Copy link
Owner

amiel commented Jun 30, 2017

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
Copy link

sukima commented May 22, 2018

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"] }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants