This repository was archived by the owner on Jul 6, 2026. It is now read-only.
⚡ Bolt: [performance improvement] Optimize DailyQuests component with useMemo - #15
Closed
ereezyy wants to merge 2 commits into
Closed
⚡ Bolt: [performance improvement] Optimize DailyQuests component with useMemo#15ereezyy wants to merge 2 commits into
ereezyy wants to merge 2 commits into
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors the DailyQuests component to memoize derived quest aggregates for performance and makes a small cleanup in TrainingEngine by changing a mutable stats map to a const declaration. Sequence diagram for DailyQuests re-render behavior with useMemosequenceDiagram
actor User
participant BrowserTimer
participant React
participant DailyQuests
User->>React: Interacts with app
React->>DailyQuests: Initial render with quests and activeTab
DailyQuests->>DailyQuests: useMemo compute filteredQuests
DailyQuests->>DailyQuests: useMemo compute completedCount
DailyQuests->>DailyQuests: useMemo compute inProgressCount
DailyQuests->>DailyQuests: useMemo compute readyToClaimTokens
DailyQuests->>DailyQuests: useMemo compute availableXP
DailyQuests-->>React: Rendered UI
loop Every 1 second
BrowserTimer->>React: timeUntilRefresh state update
React->>DailyQuests: Re-render DailyQuests
DailyQuests->>DailyQuests: Check useMemo deps for filteredQuests
DailyQuests->>DailyQuests: Check useMemo deps for aggregates
DailyQuests-->>DailyQuests: Skip recompute if quests and activeTab unchanged
DailyQuests-->>React: Rendered UI using memoized values
end
User->>React: Changes quests or activeTab
React->>DailyQuests: Re-render with new props
DailyQuests-->>DailyQuests: Recompute memoized values for changed deps
DailyQuests-->>React: Updated UI with new aggregates
Flow diagram for memoized derived quest values in DailyQuests componentflowchart TD
Quests["quests array"]
ActiveTab["activeTab state"]
subgraph MemoizedDerivations
FQ["filteredQuests = useMemo(filter by type)"]
CC["completedCount = useMemo(count completed)"]
IPC["inProgressCount = useMemo(count not completed)"]
RTCT["readyToClaimTokens = useMemo(sum tokens of completed and not claimed)"]
AXP["availableXP = useMemo(sum experience of completed and not claimed)"]
end
Quests --> FQ
ActiveTab --> FQ
Quests --> CC
Quests --> IPC
Quests --> RTCT
Quests --> AXP
FQ --> UIList["Quest list for active tab"]
CC --> UICompleted["Completed count display"]
IPC --> UIInProgress["In-progress count display"]
RTCT --> UITokens["Ready-to-claim tokens display"]
AXP --> UIXP["Available XP display"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- All the new
useMemovalues still iterate overquestsindependently; consider a singleuseMemowith onereducethat computescompletedCount,inProgressCount,readyToClaimTokens, andavailableXPin one pass to minimize work whenquestschanges. - The more complex
useMemocallbacks (e.g., forreadyToClaimTokensandavailableXP) are a bit dense inline; extracting small helper functions (or at least named predicates) could improve readability while keeping the same memoization behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- All the new `useMemo` values still iterate over `quests` independently; consider a single `useMemo` with one `reduce` that computes `completedCount`, `inProgressCount`, `readyToClaimTokens`, and `availableXP` in one pass to minimize work when `quests` changes.
- The more complex `useMemo` callbacks (e.g., for `readyToClaimTokens` and `availableXP`) are a bit dense inline; extracting small helper functions (or at least named predicates) could improve readability while keeping the same memoization behavior.
## Individual Comments
### Comment 1
<location path="src/components/DailyQuests.tsx" line_range="251-256" />
<code_context>
};
- const filteredQuests = quests.filter(q => q.type === activeTab);
+ const filteredQuests = React.useMemo(() => quests.filter(q => q.type === activeTab), [quests, activeTab]);
+
+ const completedCount = React.useMemo(() => quests.filter(q => q.completed).length, [quests]);
+ const inProgressCount = React.useMemo(() => quests.filter(q => !q.completed).length, [quests]);
+ const readyToClaimTokens = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + q.rewards.turfTokens, 0), [quests]);
+ const availableXP = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + (q.rewards.experience || 0), 0), [quests]);
return (
</code_context>
<issue_to_address>
**suggestion (performance):** Consider aggregating quest stats in a single useMemo to avoid multiple passes over `quests`.
Each of these memoized selectors (`filteredQuests`, `completedCount`, `inProgressCount`, `readyToClaimTokens`, `availableXP`) walks `quests` separately, matching the prior behavior but still incurring several full traversals per render. Consider a single `useMemo` that loops over `quests` once and returns all five values in an object to reduce overhead and centralize the aggregation logic.
```suggestion
const {
filteredQuests,
completedCount,
inProgressCount,
readyToClaimTokens,
availableXP,
} = React.useMemo(() => {
const filteredQuests = [] as typeof quests;
let completedCount = 0;
let inProgressCount = 0;
let readyToClaimTokens = 0;
let availableXP = 0;
for (const q of quests) {
if (q.type === activeTab) {
filteredQuests.push(q);
}
if (q.completed) {
completedCount += 1;
if (!q.claimed) {
readyToClaimTokens += q.rewards.turfTokens;
availableXP += q.rewards.experience || 0;
}
} else {
inProgressCount += 1;
}
}
return {
filteredQuests,
completedCount,
inProgressCount,
readyToClaimTokens,
availableXP,
};
}, [quests, activeTab]);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+251
to
+256
| const filteredQuests = React.useMemo(() => quests.filter(q => q.type === activeTab), [quests, activeTab]); | ||
|
|
||
| const completedCount = React.useMemo(() => quests.filter(q => q.completed).length, [quests]); | ||
| const inProgressCount = React.useMemo(() => quests.filter(q => !q.completed).length, [quests]); | ||
| const readyToClaimTokens = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + q.rewards.turfTokens, 0), [quests]); | ||
| const availableXP = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + (q.rewards.experience || 0), 0), [quests]); |
There was a problem hiding this comment.
suggestion (performance): Consider aggregating quest stats in a single useMemo to avoid multiple passes over quests.
Each of these memoized selectors (filteredQuests, completedCount, inProgressCount, readyToClaimTokens, availableXP) walks quests separately, matching the prior behavior but still incurring several full traversals per render. Consider a single useMemo that loops over quests once and returns all five values in an object to reduce overhead and centralize the aggregation logic.
Suggested change
| const filteredQuests = React.useMemo(() => quests.filter(q => q.type === activeTab), [quests, activeTab]); | |
| const completedCount = React.useMemo(() => quests.filter(q => q.completed).length, [quests]); | |
| const inProgressCount = React.useMemo(() => quests.filter(q => !q.completed).length, [quests]); | |
| const readyToClaimTokens = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + q.rewards.turfTokens, 0), [quests]); | |
| const availableXP = React.useMemo(() => quests.filter(q => q.completed && !q.claimed).reduce((sum, q) => sum + (q.rewards.experience || 0), 0), [quests]); | |
| const { | |
| filteredQuests, | |
| completedCount, | |
| inProgressCount, | |
| readyToClaimTokens, | |
| availableXP, | |
| } = React.useMemo(() => { | |
| const filteredQuests = [] as typeof quests; | |
| let completedCount = 0; | |
| let inProgressCount = 0; | |
| let readyToClaimTokens = 0; | |
| let availableXP = 0; | |
| for (const q of quests) { | |
| if (q.type === activeTab) { | |
| filteredQuests.push(q); | |
| } | |
| if (q.completed) { | |
| completedCount += 1; | |
| if (!q.claimed) { | |
| readyToClaimTokens += q.rewards.turfTokens; | |
| availableXP += q.rewards.experience || 0; | |
| } | |
| } else { | |
| inProgressCount += 1; | |
| } | |
| } | |
| return { | |
| filteredQuests, | |
| completedCount, | |
| inProgressCount, | |
| readyToClaimTokens, | |
| availableXP, | |
| }; | |
| }, [quests, activeTab]); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 What: Refactored the
DailyQuestscomponent to wrap derived state calculations (filteredQuests,completedCount,inProgressCount,readyToClaimTokens, andavailableXP) insideReact.useMemo.🎯 Why: Previously, these derived values were calculated directly within the component's render body using
Array.prototype.filter()andArray.prototype.reduce(). Since the component renders frequently (e.g., when thetimeUntilRefreshstate updates every second via thesetInterval), these expensive O(N) array operations were being needlessly executed on every tick, even if the underlyingquestsarray had not changed.📊 Impact: Reduces CPU overhead during the 1-second tick interval by eliminating redundant array traversals, lowering the computational cost of the main thread and resulting in smoother UI interactions, especially as the number of quests scales.
🔬 Measurement: Verified using the React DevTools Profiler by observing a reduction in render time and confirming that the memoized values do not re-evaluate when
timeRemainingupdates. Tests pass successfully.PR created automatically by Jules for task 4650786230438276965 started by @ereezyy
Summary by Sourcery
Optimize derived quest metrics in the DailyQuests component to avoid redundant computations on frequent re-renders.
Enhancements: