Normalize the URL in the baseUrl option
#579
Conversation
test/arguments.js
Outdated
| const instanceA = got.extend({baseUrl: `${s.url}/test/`}); | ||
| const {body} = await instanceA('/foobar'); | ||
| t.is(body, `/test/foobar`); | ||
| }); |
There was a problem hiding this comment.
Can you also add a test with await instanceA('foobar');?
There was a problem hiding this comment.
And a couple of tests where the baseUrl is a WHATWG URL object.
source/normalize-arguments.js
Outdated
| return options; | ||
| }; | ||
|
|
||
| module.exports.prenormalize = prenormalize; |
There was a problem hiding this comment.
prenormalize is not a word, so should be preNormalize
|
The docs needs to be updated too. |
Would be more descriptive to title the PR:
|
|
Can you comment on why it was necessary to split it out into a preNormalize step? |
Code comment or GitHub comment? |
|
Just a comment on this thread is fine. |
source/normalize-arguments.js
Outdated
| @@ -13,6 +13,47 @@ const knownHookEvents = require('./known-hook-events'); | |||
|
|
|||
| const retryAfterStatusCodes = new Set([413, 429, 503]); | |||
|
|
|||
| const preNormalize = options => { | |||
There was a problem hiding this comment.
I've split normalize-argument.js into two parts:
preNormalizehandles all the stuff which is related with static options likebaseUrl,followRedirect,hooksetc.
It is used ingot.create()to normalizedefaults.options.normalizedoespreNormalize+ handles all the stuff related with dynamic options likeurl,headers,bodyetc.
sindresorhus
changed the title
baseUrl optionbaseUrl option
|
Looking great!
|
Fixes #562