Replace `querystring` with `URLSearchParams`

[benmccann]

The querystring API is considered legacy and it's recommended to use URLSearchParams instead. I'm getting a warning when building the SvelteKit codebase with Rollup and would like to get rid of the warning

https://github.com/lukeed/polka/blob/52d5902dd11f2fb5962a439ef47799c83ba397ac/packages/polka/index.js#L3

[lukeed]

In polka@next, querystring.parse isn't used. It's only used in polka@0.5.x, and you can easily just pass the result into new URLSearchParams directly.

// example
req.query = querystring.parse('foo=bar&bar&baz=');
//=> [Object: null prototype] { 'foo': 'bar', bar: '', baz: '' }

// override
req.query = new URLSearchParams(req.query);
//=> URLSearchParams { 'foo' => 'bar', 'bar' => '', 'baz' => '' }

[benmccann]

Hmm, I'm not sure that's right. When I run pnpm install from the kit root directory I see it used

File node_modules/.pnpm/@polka+url@1.0.0-next.15/node_modules/@polka/url/build.js

Line 36:

                query = qs.parse(search.substring(1));

Looks like it's coming from here:

https://github.com/lukeed/polka/blob/e0e131dd28b03e26652a96b9a1428c017209e187/packages/url/index.js#L36

[lukeed]

Oh right, sorry was thinking about polka itself.

Sorry, definitely not changing that. Same suggestion from above still applies.

[lukeed]

Upcoming framework might be more appealing as a base for something like SK, btw. It's meant to handle different environments seamlessly, which is a good fit for a multi-adapter consumer

[benmccann]

Would you accept a PR to change it or is there a reason to avoid updating it?

The suggestion won't work for me because Kit isn't calling @polka/url directly, so there's no easy way to avoid having that line of code being called.

I also wonder if there's a possibility of changing the way that it's imported. Could we updated it so that instead of:

import * as qs from 'querystring';
...
query = qs.parse(search.substring(1));

It could instead do:

import { parse } from 'querystring';

I haven't tested, but think that might be what Rollup is complaining about. I don't know if there's some ESM/CJS compatibility thing that might break though.

[lukeed]

It wouldn't make any difference to Rollup.

My suggestion still applies. Just make the change at the first point of contact outside of the Polka handler, so like inside your first .use() middleware.

Not and can't change it because Polka is meant to work with the Express ecosystem, and req.query is always a plain object. Changing that is breaking, not to mention it's slower and decodes slightly differently iirc.

[benmccann]

I don't understand how changing anything inside adapter-node would stop polka/url from being included in the SvelteKit code

I wasn't suggesting to change the API. I'd convert back from a URLSearchParam to a pojo

[lukeed]

No thank you