fix(middleware): Correlate responses by index when server does not re… #56
Conversation
| }, | ||
| method: 'POST', | ||
| }); | ||
|
|
||
| const req1 = mockReq(1); | ||
| const req2 = mockReq(2); | ||
| const req3 = mockReq(3); |
There was a problem hiding this comment.
The test description says duplicated ids, but the IDs were not actually duplicated :)
| @@ -63,7 +101,7 @@ describe('batchMiddleware', () => { | |||
| matcher: '/graphql/batch', | |||
| response: { | |||
| status: 200, | |||
| body: [{ data: {} }, { id: 2, data: { ok: 2 } }], | |||
There was a problem hiding this comment.
Changed to preserve the original error message - Otherwise it will hit the new path, because a response does not have an id. Alternatively I can change it to return an ID that has not been requested
| @@ -174,9 +174,16 @@ function sendRequests(requestMap: BatchRequestMap, next, opts) { | |||
| throw new Error('Wrong response from server'); | |||
| } | |||
|
|
|||
| batchResponse.payload.forEach(res => { | |||
| const responseHasIds: boolean = batchResponse.payload.every(response => response.id); | |||
| if (!responseHasIds && ids.length !== batchResponse.payload.length) { | |||
There was a problem hiding this comment.
This check could be worthwile to have in any case - I guess the number of responses should always be the same as the number of requests?
There was a problem hiding this comment.
Who knows. Any server may have bugs, and we should catch this somehow and reject with error by spec.
|
By the way, this closes #55 |
|
Awesome work! 👍 New package version will be published from minute to minute. |
|
No problem, thanks for creating this library 🌮 ! |
…turn ID