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

Stringify with "arrayFormat: comma" encodes objects inside arrays as "[object Object]" #378

Open
FreekyMage opened this issue Aug 11, 2020 · 30 comments

Comments

@FreekyMage
Copy link

FreekyMage commented Aug 11, 2020

Tested on version 6.9.4

Stringify with arrayFormat: comma encodes objects inside arrays as [object Object]

I took this code from the examples:

qs.stringify({ a: { b: { c: 'd', e: 'f' } } });
// 'a[b][c]=d&a[b][e]=f'

and added an array around the inside object

qs.stringify({ a: { b: [{ c: 'd', e: 'f' }] } }, { encode: false, arrayFormat: 'comma' })
// 'a[b]=[object Object]'

The inside data is lost this way.

I tried the allowDots setting too, but that didn't fix anything.

I'm not sure what the output should be like exactly, but this is unusable right now.
In my project I did Array.map(p => ${p.ID}::${p.Value}), but that's very specific and I'm unsure if the :: is a good seperator.

@ljharb
Copy link
Owner

ljharb commented Jan 13, 2021

I agree the current behavior is unusable. However, it is really unclear to me what the format should be, because the "comma" format doesn't work the same way normal query strings work. In other words, i have no idea whatsoever how an object inside an array would be represented in that format. Any suggestions?

cc @daggerjames @bryanlarsen who originally implemented/requested this format

@ls-urs-keller
Copy link

I guess arrayFormat: 'comma' should only be applied to an array with primitives e.g. number, string ...

@ljharb
Copy link
Owner

ljharb commented Sep 22, 2022

@ls-urs-keller one possibility is that qs could throw if it encounters a non-primitive when stringifying arrayFormat comma, but maybe there's something useful to be done instead.

@ls-urs-keller
Copy link

ls-urs-keller commented Sep 22, 2022

My need is compact query strings when possible, but still, have it work correctly for nested object arrays. I'm using this workaround for the moment:

const stringifyOpts = {
  allowDots: true,
  filter: (_prefix: string, value?: unknown): unknown => {
    const isNonNullPrimitive = (x?: unknown): boolean => !!x && Object(x) !== x;
    return Array.isArray(value) &&
      value.filter((v) => !isNonNullPrimitive(v) || v.indexOf(',') > -1).length === 0
      ? value.join(',')
      : value;
  },
};
stringify(queryParams, stringifyOpts)

@ljharb
Copy link
Owner

ljharb commented Sep 22, 2022

You're missing a few primitive types; i'd suggest doing Object(x) === x to determine if something's an object or not.

@ls-urs-keller
Copy link

ls-urs-keller commented Sep 22, 2022

You're missing a few primitive types; i'd suggest doing Object(x) === x to determine if something's an object or not.

Thank you. How about now?
Just wondering if this remark is valid here as well https://github.com/ljharb/qs/blob/main/lib/stringify.js#L51 ?

@nandi95
Copy link

nandi95 commented Sep 22, 2022

@ls-urs-keller one possibility is that qs could throw if it encounters a non-primitive when stringifying arrayFormat comma, but maybe there's something useful to be done instead.

This is a good idea as people will likely get tripped up by this

@ls-urs-keller
Copy link

@ls-urs-keller one possibility is that qs could throw if it encounters a non-primitive when stringifying arrayFormat comma, but maybe there's something useful to be done instead.

This is a good idea as people will likely get tripped up by this

I would prefer a mixed mode as per my comments above.

@ljharb
Copy link
Owner

ljharb commented Sep 16, 2023

Howdy, folks that care about the comma array format!

My intuition is that what you want most is either to be able to round-trip things between parse and stringify (which requires [] brackets to be added on one-item arrays in stringify, or on all of them); or, to match the API of a specific server implementation.

If the latter, can folks please describe exactly what they're using and what format it requires? If it supports what qs calls brackets, indices, or repeat, please also explain why you need to use comma over those.

@runspired
Copy link

Most likely every user of JSON:API

https://jsonapi.org/format/#fetching-includes

The value of the include parameter MUST be a comma-separated (U+002C COMMA, “,”) list of relationship paths. A relationship path is a dot-separated (U+002E FULL-STOP, “.”) list of relationship names. An empty value indicates that no related resources should be returned.

@ljharb
Copy link
Owner

ljharb commented Sep 23, 2023

@runspired so, a single array, represented as .join(','), effectively? in that example presumably a relationship path can't contain a comma itself?

@runspired
Copy link

@ljharb correct. E.g. ?include=foo,bar.baz,bem,bem.baz.bar&otherParam=1

@ljharb
Copy link
Owner

ljharb commented Sep 23, 2023

@runspired and if there's an array of length 1, does it need to be include[]=foo so the other side knows it's meant to be an array? or does the JSON API format already guarantee it's an array no matter how many items are in it?

@runspired
Copy link

Guarantees it's an array regardless of how many are in it

@runspired
Copy link

@ljharb btw you got me thinking and I am going to bring up to the spec authors that maybe include=foo,bar,baz should become include[]=foo,bar,baz so that its always clear to parsers

@ljharb
Copy link
Owner

ljharb commented Oct 2, 2023

It 1000% should be :-)

@jelhan
Copy link

jelhan commented Oct 3, 2023

@ljharb btw you got me thinking and I am going to bring up to the spec authors that maybe include=foo,bar,baz should become include[]=foo,bar,baz so that its always clear to parsers

This is a misunderstanding of the meaning of square brackets in query parameters as defined by JSON:API specification. For JSON:API specification query brackets do not indicate a list. Instead they build a query parameter family: A set of related but distinct query parameters.

This is an example given in the specification:

/?page[offset]=0&page[limit]=10

Please see the section on query parameter families in the specification for details.

@runspired
Copy link

@jelhan this isn't about what the meaning is currently, its about the current meaning being unparseable without intimate knowledge of the schema the url is going to expand to.

@ljharb
Copy link
Owner

ljharb commented Oct 4, 2023

@jelhan that sounds like a design flaw in the specification then, since there needs to be a clear way to determine if something is a single item or a list of one item, and brackets are otherwise the universal mechanism to do that for query strings.

@jelhan
Copy link

jelhan commented Oct 4, 2023

I would recommend disabling array parsing ({ parseArrays: false }) when using qs with an API which implements the JSON:API specification. If the value should be parsed as an array or a single value depends on the processing rules defined for the specific query parameter by the base specification, an applied profile or extension, or the implementation itself. This cannot be derived from the encoding itself.

In addition, I would recommend not relying on { arrayFormat: 'comma' } for stringifying. Query parameters include, sort, and fields defined by the base specification use comma-separated lists. But query parameters defined by applied profiles, extensions, or implementation may encode lists differently. Therefore relying on generic rules in query parameter parser is risky.

One may argue that it's a design flaw not being able to derive from encoding schema if value should be parsed as a list. But one could make the same argument for not being able to derive from encoding schema if it should be parsed as s boolean, number, null, date, or even RSQL. I guess it's on everyone individually to judge.

I don't think this issue is the correct place to discuss design decisions of JSON:API specification. I invite you to continue that discussion in JSON:API discussion forum or in an issue at JSON:API GitHub repository.

@ljharb
Copy link
Owner

ljharb commented Oct 5, 2023

I think the same argument holds! It's very much a flaw that these things aren't part of the encoding, but a list is far more fundamental than a type.

I appreciate your context; I'm trying to figure out how and when the comma format is useful so it can better meet those use cases. It seems that JSON:API is not a valid use case for the comma format, so I return to my original question:

Howdy, folks that care about the comma array format!

My intuition is that what you want most is either to be able to round-trip things between parse and stringify (which requires [] brackets to be added on one-item arrays in stringify, or on all of them); or, to match the API of a specific server implementation.

If the latter, can folks please describe exactly what they're using and what format it requires? If it supports what qs calls brackets, indices, or repeat, please also explain why you need to use comma over those.

@ihor-zinchenko
Copy link

same for me

{query: '', page: '1', pageSize: '20', sort: '[object Object]'}

Original:

image

@ihor-zinchenko
Copy link

encode: false,

im using qs for query params on FE part of app.
Why i need comma - its because query str length limits

@ljharb
Copy link
Owner

ljharb commented Nov 24, 2023

@ihor-zinchenko can you elaborate? query strings have a massive limit; if you’re approaching it you should be using POST and not GET.

@ihor-zinchenko
Copy link

@ljharb i use it for deeplinking on my FE app, i know about limits but i wont had so long string. The main problem that Object becames Object Object with comma

@ljharb
Copy link
Owner

ljharb commented Nov 25, 2023

can you share an example? deep linking just requires a path which usually isn’t particularly long.

@ihor-zinchenko
Copy link

ihor-zinchenko commented Nov 25, 2023

@ljharb sure
Example is simple, i have an object with data, like this

{
  filters: {
    author: ['Author 1', 'Author 2'],
    // other filters
  },
  sort: [
    {
      publication_date: 'asc'
    }
  ],
  page: 1,
  perPage: 20,
  // other params
}

And i just need to convert this object to URL what QS do really good, but i need save the data length so i need some kind shortify the url. comma really helpful, so this object converts to
search?filters[author]=Author 1,Author 2&query=&publicationState=live&page=1&pageSize=20&sort=[object Object]
with comma separator, and its so much longer then without:
search?filters[author][0]=Author 1&filters[author][1]=Author 2&query=&publicationState=live&page=1&pageSize=20&sort[0][publication_date]=desc but in comma case sort is [object Object]

@ljharb
Copy link
Owner

ljharb commented Nov 25, 2023

Thanks, that sounds like a really complex URL scheme.

I wonder if it might make more sense to translate the author names into numeric IDs, and similar data normalization techniques?

@ihor-zinchenko
Copy link

@ljharb its make sense in filters case but for sorting is not

@ljharb
Copy link
Owner

ljharb commented Nov 26, 2023

True. Either way it seems like a better approach for you is to use a custom encoder and/or filter function to make your query string shorter rather than trying to use comma for that purpose. Thanks for the use case!

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