perf: Implement request coalescing (single-flight) for user contributions fetching#5286
Conversation
|
@MasterJi27 is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds in-flight request coalescing to reduce duplicate concurrent GitHub contribution fetches for the same cache key.
Changes:
- Introduces a module-level map to track active contribution fetch promises by cache key.
- Wraps the uncached fetch function with a coalescing loader that reuses an existing in-flight promise.
- Routes both cache-bypass and cached
getOrSetfetches through the coalescing loader.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * DATA FETCHING | ||
| * ========================================================================== */ | ||
|
|
||
| const activeContributionsPromises = new Map<string, Promise<ExtendedContributionData>>(); |
| const coalescedLoad = () => { | ||
| let pending = activeContributionsPromises.get(key); | ||
| if (!pending) { | ||
| pending = load().finally(() => { | ||
| activeContributionsPromises.delete(key); | ||
| }); | ||
| activeContributionsPromises.set(key, pending); | ||
| } | ||
| return pending; | ||
| }; |
| * DATA FETCHING | ||
| * ========================================================================== */ | ||
|
|
||
| const activeContributionsPromises = new Map<string, Promise<ExtendedContributionData>>(); |
| pending = load().finally(() => { | ||
| activeContributionsPromises.delete(key); | ||
| }); |
9780a93 to
8b25072
Compare
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for putting this together. The code looks clean and solves the issue effectively.
Looks solid. I'll go ahead and approve.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and everything looks solid. The code is readable, well-structured, and aligns with the project conventions.
I'll go ahead and approve this PR. Thanks again for the contribution!
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I went through the changes and the overall approach looks good, but there are a few issues that should be addressed before this can be merged. Most of the concerns are related to correctness and maintainability.
- There are merge conflicts with the base branch. Please resolve them to ensure existing functionality isn't broken.
Once these issues are addressed, I'll be happy to take another look. Thanks again for the contribution.
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for the contribution. It looks like there are merge conflicts with the base branch. Please rebase and resolve the conflicts so we can proceed with testing and merging. Thanks!
b0da805 to
5b709bb
Compare
|
@Aamod007 all done form my side |
|
Hi @Aamod007, All CI checks are passing from my side. The only remaining issue is the Vercel deployment authorization, which appears to be a repository/configuration permission issue rather than a code issue. Could you please review and approve the PR if everything looks good? PR: #5286 Thanks. |
|
Review Update |
|
Hi @Aamod007, I've rebased the branch and resolved all merge conflicts. All required CI checks are passing successfully. Could you please re-review the PR and dismiss the previous change request if everything looks good? Thanks. |
98a173f to
67b8e53
Compare
Aamod-Dev
left a comment
There was a problem hiding this comment.
Thanks for resolving the merge conflicts! Approving.
|
@Aamod007 Thank you for reviewing and approving my PRs. Currently, I have the following approved PRs open: #5364 – bug: Sanitize MongoDB operators from user-tracking payloads Could someone from the team please authorize the Vercel deployment and merge these PRs when convenient? It would help me continue working on additional assigned issues without having multiple approved PRs pending. Thank you! |
|
🎉 Congratulations @MasterJi27! Your PR has been successfully merged. 🚀 Thank you for contributing to CommitPulse. Your work helps us build a better tool for the community.
Keep building! 💻✨ |
Resolves #5237. Introduces promise coalescing using an activeContributionsPromises map to prevent duplicate fetches for simultaneous concurrent requests.