Bump graphql-js-client to 0.11.0 #638
Conversation
test/fetch-mock-helper.js
Outdated
| @@ -0,0 +1,8 @@ | |||
| export default function fixtureWithHeaders(fixture) { | |||
There was a problem hiding this comment.
Do we need a test to test this helper function?
There was a problem hiding this comment.
That doesn't seem like a bad idea. Technically, if this breaks, it could invalidate a whole whack of tests. I think tho, with the more recent change to the client (ignoring headers and trying to parse json and falling back to text), this method will probably become unnecessary. @rebeccajfriedman, is that correct?
There was a problem hiding this comment.
@minasmart you are right. We are going to hold off on merging that PR in graphql-js-client because we aren't so happy with the resultant code, and we were struggling to come up with a better approach.
rebeccajfriedman
force-pushed
the
bump_graphql_js_client
branch
from
d9d8eb8
to
3672098
Compare
|
|
||
| export default function fetchMockPostOnce(fetchMock, apiUrl, fixture) { | ||
| fetchMock.postOnce(apiUrl, fixtureWithHeaders(fixture)); | ||
| } |
There was a problem hiding this comment.
I'm curious as to what this wrapper is helpful for (I may be missing some context). Is this required due to bumping the graph package, or is it just a syntactically preferred way to write these tests?
There was a problem hiding this comment.
Discussed offline :) [but largely syntax preference]
Forward fix for #614 as it includes Shopify/graphql-js-client#121