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

does json option support all json values? #2013

Closed
samzilverberg opened this issue Mar 20, 2022 Discussed in #2008 · 5 comments
Closed

does json option support all json values? #2013

samzilverberg opened this issue Mar 20, 2022 Discussed in #2008 · 5 comments
Labels
enhancement This change will extend Got features ✭ help wanted ✭

Comments

@samzilverberg
Copy link
Contributor

samzilverberg commented Mar 20, 2022

I was trying to use GOT (latest) with Typescript to write some "niche" tests that send some json that probably only people trying to break your server would use.

values such as: true, false, null, "123", 1, [1,2,3].

But noticed that GOT only supports Record<string, any>.

here's the source of it:

got/source/core/options.ts

Lines 1356 to 1365 in d95ceea

set json(value: Record<string, any> | undefined) {
assert.any([is.object, is.undefined], value);
if (value !== undefined) {
assert.undefined(this._internals.body);
assert.undefined(this._internals.form);
}
this._internals.json = value;
}

This means only objects and array can be passed in.
Shouldnt json support all valid json values?

I tried looking up documenation or issues to understand why this limitation exists but didn't find any :(

If this limitation is here to stay, then is there a way to use GOT to send the other json values?

@szmarczak szmarczak added enhancement This change will extend Got features ✭ help wanted ✭ labels Mar 20, 2022
@szmarczak
Copy link
Collaborator

In that case value should be typed as any.

@samzilverberg
Copy link
Contributor Author

@szmarczak dya mean the type for the set json(...) function?

it can be typed better than any :) , for example:

set json(value: Record<string, any> | number | string | boolean | undefined) { 

@szmarczak
Copy link
Collaborator

https://github.com/microsoft/TypeScript/blob/45c1e5880936a56b61071d14c929674e6f595c93/lib/lib.es5.d.ts#L1066

TypeScript ships JSON.stringify with any so let's stick with that.

@samzilverberg
Copy link
Contributor Author

@szmarczak i made the proposed any change in #2015

I wanted to add some tests but wasn't sure which test file would be most appropriate.
I didn't find any exising tests of what json option should and shouldn't accept.

@samzilverberg
Copy link
Contributor Author

@szmarczak i'm closing this issue.

note that sindresorhus suggested to use unknown for the return value of get json

#2015 (comment)

i can confirm that using unknown instead of any compiles and all tests run (on my machine).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This change will extend Got features ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

2 participants