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

convertFormUrl behaves unexpectedly with arrays #26

Closed
sushain97 opened this issue Jun 16, 2018 · 4 comments
Closed

convertFormUrl behaves unexpectedly with arrays #26

sushain97 opened this issue Jun 16, 2018 · 4 comments

Comments

@sushain97
Copy link

https://runkit.com/embed/gh39bs0gaifx:

var wretch = require("wretch");
var qs = require("qs");

const convertFormUrl = (formObject) => {
    return Object.keys(formObject)
        .map(key =>
            encodeURIComponent(key) + "=" +
            `${ encodeURIComponent(typeof formObject[key] === "object" ? JSON.stringify(formObject[key]) : formObject[key]) }`)
        .join("&")
};

const obj = {'a': [1, 2]};
console.log(convertFormUrl(obj), unescape(convertFormUrl(obj)));
console.log(qs.stringify(obj), unescape(qs.stringify(obj)));

==>

"a=%5B1%2C2%5D"
"a=[1,2]" // wretch
"a%5B0%5D=1&a%5B1%5D=2"
"a[0]=1&a[1]=2" // qs

I expect something like the second.

@elbywan
Copy link
Owner

elbywan commented Jun 16, 2018

Hey @sushain97!

Just realized that you are talking about the formUrl method! Disregard my answer below, sorry 😉

"a=[1,2]" // wretch

Hmm I don't get this with wretch. I get : a=1&a=2

I expect something like the second.

Why? Wretch uses the standard class URLSearchParams to encode query strings.
Extract from the node.js documentation (same spec. on MDN):

const params = new URLSearchParams();
params.append('foo', 'bar');
params.append('foo', 'baz');
params.append('abc', 'def');
console.log(params.toString());
// Prints foo=bar&foo=baz&abc=def

If you want to use any custom encoding function it's pretty easy anyway 😉 :

https://runkit.com/embed/74fh6tbh1mo2

var wretch = require("wretch")
var { URLSearchParams } = require('url')

const urlParams = new URLSearchParams()
urlParams.append('a', 1)
urlParams.append('a', 2)

console.log(
    'Standard URLSearchParams: ' + 
    urlParams.toString()
)
// > "Standard URLSearchParams: a=1&a=2"

const obj = { a: [1, 2] } 
wretch().polyfills({ URLSearchParams })

console.log(
    'Wretch with the standard URLSearchParams: ' + 
    wretch('/').query(obj)._url
)
// > "Wretch with the standard URLSearchParams: /?a=1&a=2"

console.log(
    'Wretch using your own custom string: ' + 
    wretch('/').query('a=[1,2]')._url
)
// > "Wretch using your own custom string: /?a=[1,2]"

@sushain97
Copy link
Author

sushain97 commented Jun 16, 2018

No worries; I was light on the prose in my original question.

@elbywan
Copy link
Owner

elbywan commented Jun 16, 2018

Okay, got a proper answer now!

I had a look at qs, and it does encode query strings pretty well, and it supports a lot of different outputs when encoding an array.
But my goal is to keep the codebase smallest as possible, so I cannot re-implement the same mechanisms inside wretch.

But the good thing is that if you want to have the same output as qs it is relatively easy to achieve:

wretch().formUrl(qs.stringify({ a: 1, a: 2 })

On a side note, wretch does not encode form urls properly indeed. It should do the same as for query strings, using URLSearchParams which is the standard.

For instance this form, when posted redirects to /?a=1&a=2.

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
</head>
<body>
    <form action="/" method="GET">
        <input type="text" name="a"/>
        <input type="text" name="a"/>
        <input type="submit" value='submit'/>
    </form>
</body>
</html>

I consider that the browsers behaviour is the standard one, so I am going to align wretch output with that!

@sushain97
Copy link
Author

But my goal is to keep the codebase smallest as possible

+💯

I consider that the browsers behaviour is the standard one, so I am going to align wretch output with that!

Sounds good! Unfortunately, that doesn't align with what I want, but you're right -- standard is best. Thanks for the workaround and your work on this great library.

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

2 participants