elbywan / wretch

A tiny wrapper built around fetch with an intuitive syntax. :candy:
MIT License
4.83k stars 98 forks source link

convertFormUrl behaves unexpectedly with arrays #26

Closed sushain97 closed 6 years ago

sushain97 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

elbywan commented 6 years ago

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 commented 6 years ago

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.