Axios 16 compat #4
Conversation
Default removed in node-fs-extra / json-file 3.0.0: See: https://github.com/jprichardson/node-fs-extra/blob/master/CHANGELOG.md\#removed
lib/RequestMiddleware.js
Outdated
| cassette.originalResponseData.fixture = true | ||
| return resolve(cassette.originalResponseData) | ||
| if (arguments.length === 3) { // axios <= 0.12 | ||
| var resolve = arguments[0]; |
There was a problem hiding this comment.
can you remove the extra semi colons?
| @@ -11,7 +11,8 @@ function key(axiosConfig) { | |||
| method: axiosConfig.method, | |||
| data: axiosConfig.data, | |||
| headers: _.omit(axiosConfig.headers, [ | |||
| 'Content-Length', 'content-length' | |||
| 'Content-Length', 'content-length', | |||
| 'User-Agent', 'user-agent' | |||
There was a problem hiding this comment.
do you know if axios stopped injecting its own user-agent header?
that was one of the reasons I needed some of those workarounds.
There was a problem hiding this comment.
@nettofarah this change was made quite some time ago: axios/axios@db85c7b - Axios only injects its own User-Agent header if none is set.
There was a problem hiding this comment.
@nettofarah I obviously can't read! Your original comment stated the exact same thing 😄
Removing the User-Agent is an alternate strategy that I decided to pursue after testing React components with axios-vcr + Axios + jsdom. In order to simulate a browser environment, jsdom actually provides an implementation of XmlHttpRequest, which causes axios to use the xhr adapter.
Thanks to CORS, the addition of a custom User-Agent means that the request is no longer classified as "simple" but rather one that needs to preflighted. As such, an extra OPTIONS request is fired before the actual request (and in my particular use case, this is something our server doesn't support). / @NudelSieb
If we decide to reinstate the code injecting the User-Agent: axios-vcr header, we would first need to check if the original adapter is http.
There was a problem hiding this comment.
If we decide to reinstate the code injecting the User-Agent: axios-vcr header, we would first need to check if the original adapter is http.
If I understand things correctly, this might be the safer option here.
| @@ -21,11 +21,11 @@ function writeAt(filePath, jsonPath, value) { | |||
|
|
|||
| return fs.readJson(filePath).then(function(json) { | |||
| _.set(json, jsonPath, value) | |||
| return fs.writeJson(filePath, json) | |||
| return fs.writeJson(filePath, json, { spaces: 2 }) | |||
test/axios_vcr_test.js
Outdated
| assert.deepEqual(fixture.originalResponseData, _.omit(res, 'fixture')) | ||
| done() | ||
|
|
||
| VCR.ejectCassette(path) | ||
| }).catch(err => { console.log(err); done() }) |
There was a problem hiding this comment.
why would you remove the catch call here?
we want to make sure tests don't hang if they fail.
myabc
force-pushed
the
axios-16-compat
branch
from
4996cd4
to
3683fa9
Compare
|
|
Using this branch and axios 16 I have a problem. jsonPaths do not match between request and response {
"url":"https://launchlibrary.net/1.2/launch",
"method":"get",
"headers":{
"common":{
"Accept":"application/json, text/plain, */*"
},
"delete":{
},
"get":{
},
"head":{
},
"post":{
"Content-Type":"application/x-www-form-urlencoded"
},
"put":{
"Content-Type":"application/x-www-form-urlencoded"
},
"patch":{
"Content-Type":"application/x-www-form-urlencoded"
}
}
}And during response {
"url":"https://launchlibrary.net/1.2/launch",
"method":"get",
"headers":{
"Accept":"application/json, text/plain, */*"
}
} |
|
also for some reason it doesn't take |
|
@soswow I would prefer that the library is backward compatible. But I'm ok with issuing a new major release to only support newer versions of axios |
Some additions to @teppeis's PR #3 (mostly to address this comment: #3 (comment))