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

Consider use of qs library to serialize data into the url #7835

Closed
snewcomer opened this issue Jan 2, 2022 · 6 comments
Closed

Consider use of qs library to serialize data into the url #7835

snewcomer opened this issue Jan 2, 2022 · 6 comments

Comments

@snewcomer
Copy link
Contributor

Right now we are serializing the data into the url with an implementation mirroring ajax. See serializeQueryParams.

https://github.com/ljharb/qs

e.g.

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

ref #7824
See - https://github.com/ljharb/qs#rfc-3986-and-rfc-1738-space-encoding

@snewcomer
Copy link
Contributor Author

snewcomer commented Jan 2, 2022

This library would also resolve #7721

assert.equal(qs.stringify({ a: '' }), 'a=');

@snewcomer
Copy link
Contributor Author

This is likely more bytes than what we could write ourselves. However, this would make us "more robust" and less prone to issues for our users.

@acorncom
Copy link
Contributor

acorncom commented Jun 29, 2022

I was looking into this package myself yesterday, was a bit concerned though looking at the bundle size for it (https://bundlephobia.com/package/qs@6.11.0)

@acorncom
Copy link
Contributor

https://github.com/sindresorhus/query-string might also be of interest

@runspired
Copy link
Contributor

For the RequestManager RFC emberjs/rfcs#860 url building is brought out of adapter in a way that this is mostly a consuming app concern. I don't think we need to change what the adapters do today; however, we may want to consider a new default for the url-builder utility.

qs being a cjs module (and a rather large one at that) is a non-starter and query-string hides its size with dependencies. There's probably one that is built for browsers only that is fairly tiny, we should research.

@runspired
Copy link
Contributor

we've implemented a simple query string builder that's optional for the request builders work. If someone wants to use qs (highly discouraged fwiw) all they need to do is install qs and use it to build their query params for the url they pass to request

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