-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
batch treats same query with different request variables as a duplicate #31
Comments
Confirm the bug! For backward compatibility needs to write a new middleware with name In the
|
return new Promise((resolve, reject) => { | |
const relayReqId = req.getID(); | |
const { requestMap } = singleton.batcher; | |
const requestWrapper: RequestWrapper = { | |
req, | |
completeOk: res => { | |
requestWrapper.done = true; | |
resolve(res); | |
requestWrapper.duplicates.forEach(r => r.completeOk(res)); | |
}, | |
completeErr: err => { | |
requestWrapper.done = true; | |
reject(err); | |
requestWrapper.duplicates.forEach(r => r.completeErr(err)); | |
}, | |
done: false, | |
duplicates: [], | |
}; | |
if (requestMap[relayReqId]) { | |
/* | |
I've run into a scenario with Relay Classic where if you have 2 components | |
that make the exact same query, Relay will dedup the queries and reuse | |
the request ids but still make 2 requests. The batch code then loses track | |
of all the duplicate requests being made and never resolves or rejects | |
the duplicate requests | |
https://github.com/nodkz/react-relay-network-layer/pull/52 | |
*/ | |
requestMap[relayReqId].duplicates.push(requestWrapper); | |
} else { | |
requestMap[relayReqId] = requestWrapper; | |
} | |
}); |
relayReqId
should be removedrequestMap
renamed torequestList
and usedpush
methodduplicates
logic should be removed for simplicity (and may be added in future). Also should be removed in linerequest.duplicates.forEach(r => r.completeOk(res));
In sendRequests
method
- should be changed
ids
on array order payload.id
should be removed and also should be rewritten on array orderreact-relay-network-modern/src/middlewares/batch.js
Lines 198 to 205 in 331a2f7
batchResponse.json.forEach((payload: any) => { if (!payload) return; const request = requestMap[payload.id]; if (request) { const res = createSingleResponse(batchResponse, payload); request.completeOk(res); } });
In my apps I do not use batching (all requests sending in parallel - page loads more progressive and feels more alive). So I'm not redy to write a new batch
middleware. But with pleasure review and merge PR if somebody make it. Help wanted!
just curious - why not alter |
+1 Any chance this will be fixed soon? This middleware is largely unusable because of this issue. I use with the same graphql fragment to render multiple configurations of the same widget on the same page. All widgets receive the same data which is bad. |
If somebody sends Pull Request with fix I'll merge it. Sorry, I haven't bandwidth for this issue to do it myself. |
Sure! |
react-relay-network-modern/src/middlewares/batch.js
Line 139 in 331a2f7
I have a refetch query in modern that is being run on two instances of the same component, each with different variables on the request. Within the batch middleware, the relayReqId is identical and the same name as the refetch query. However, the fetch variables are different, but any subsequent request in the same batch that has different variables seems to only get treated as a dup and not fetched.
The text was updated successfully, but these errors were encountered: