Bugfix/transformResponse - fixes #6 #8
Conversation
… to get response as JSON
…phql-rest-router into bugfix/transformResponse
Route.ts
Outdated
| const { transformResponse } = axios.defaults; | ||
|
|
||
| if (!transformResponse) { | ||
| return []; |
There was a problem hiding this comment.
This should always exist right? Granted transformResponse is included in axios as a part of the defaults
There was a problem hiding this comment.
Or are the defaults being overridden?
There was a problem hiding this comment.
Yeah, it should technically exist as an AxiosTransformer[] seen here: https://github.com/axios/axios/blob/master/lib/defaults.js#L57-L65.
I can't think of a case where it wouldn't be available, but Typescript was yelling at me because the types are AxiosTransformer(a function) | AxiosTranformer[] | undefined.
So I could remove getDefaultTransformResponse and do
this.transformResponseFn = axios.defaults.transformResponse as AxiosTransformer[]Does that look better? It seems like a lot of work just to use a function that we already have available here: https://github.com/Econify/graphql-rest-router/blob/master/Route.ts#L52-L58.
There was a problem hiding this comment.
I ended up using the axios.default transform always and type casting it because I can't think of a case where it wouldn't be available.
|
Closing, will address in #53 instead |
This commit fixes #6.
***** EDIT ****
Addressing 1 below: I am now typecasting transformResponse from the axios.defaults so the function I wrote is no longer included. Addressing 2-4 below: I removed the integration test in favor of unit testing. So logs are gone and questions about tests are in regards to an integration test that is no longer in this PR.
There's a couple of things that should be noted/addressed before merging:
AxiosTransformer | AxiosTransformer[] | undefined. This means that I wrote a function to pull out the default transformer and always return it as AxiosTransformer[]. That seems a bit overkill but the other option was to type cast it as AxiosTransformer[] or rewrite the above function ourselves.#makeRequest. I don't know if this is the best way to do it but I'm happy to change it if needed. If you have suggestions on how to effectively test thattransformResponsereturns JSON if the response is stringified JSON with Sinon or Chai, let me know.done()to tell the test runner it's completed. However, when this test fails, it times out and I'm unsure where to catch the error to calldone(error). Any ideas?Thanks!