Change stringifying options behavior #297
Conversation
Soon the json option will also stringify. All tests that currently rely on the 'parse JSON res option' but don't shouldn't stringify their body as JSON have to be updated.
Since it only deals with parsing. Stringifying tests are in test/post.js
Removes falling back to stringifying plain objects as querystrings. Cleans up general input validation. Adds form and json option input validation. Limits the use of `body` shorthand to reading only. Moves method setting and fallback closer together.
|
@floatdrop What do you think of these changes? See my proposal in #174 (comment) |
👍 Wait until this lands first though. |
|
I got an avocado rainbow 😲 ✊ ❤️! |
index.js
Outdated
| } | ||
|
|
||
| opts.method = opts.method || 'POST'; | ||
| if ((opts.form || opts.json) && !(isObj(body) && typeof body !== 'function')) { |
There was a problem hiding this comment.
Is typeof body !== 'funciton' is necessary? Looks like isPlainObj(function() {}) returns false.
There was a problem hiding this comment.
Nice catch. I was confusing is-obj and is-plain-obj.
index.js
Outdated
| } | ||
|
|
||
| opts.method = opts.method || 'POST'; | ||
| if ((opts.form || opts.json) && !(isObj(body) && typeof body !== 'function')) { | ||
| throw new TypeError('options.body must be a plain Object'); |
There was a problem hiding this comment.
options.body must be a plain Object when options.form or options.json is used
index.js
Outdated
|
|
||
| opts.method = (opts.method || 'GET').toUpperCase(); | ||
| opts.method = opts.method || 'POST'; |
There was a problem hiding this comment.
opts.method should be uppercased in this if-branch too.
|
Thank you for yet another excellent pull request @alextes. I'd like to invite you to join the project if you're interested? 🙏 |
|
@sindresorhus 🙇 🙇 . You just made my day. And it's only 10 AM! |
Changes the behavior of the stringifying options.
formorjsonis truejsonis trueformis trueresas JSON ifformandjsonare trueThings I changed because I found the code hard to deal with, and confusing without.
bodyvariable. It's a shorthand for the body passed by the user.opts.bodyis what we mutate into the body we want to send.As always details are in the commit messages.
Sidenotes: I went for minimal change based on the proposal from #174. I think this can be quite a bit cleaner still. I'm considering a second PR. Specifically what I feel could be better:
Fixes #174