fix: blocks page stale auto-update, timer drift, and Promise anti-pattern#601
Merged
samufacanha2 merged 3 commits intodevelopfrom Mar 9, 2026
Merged
Conversation
…tern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
WalkthroughAuto-update now refetches both total and yesterday statistics; card age is derived from query-provided dataUpdatedAt timestamps; CardContent accepts a required dataUpdatedAt prop; API calls removed Promise-constructor anti-pattern and now throw Errors on failure. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(135,206,235,0.5)
participant Browser
end
rect rgba(144,238,144,0.5)
participant BlocksComponent
end
rect rgba(255,182,193,0.5)
participant ReactQuery
end
rect rgba(255,255,0,0.5)
participant APIService
end
Browser->>BlocksComponent: load /blocks
BlocksComponent->>ReactQuery: useQuery totalStatisticsCall
BlocksComponent->>ReactQuery: useQuery yesterdayStatisticsCall
ReactQuery->>APIService: fetch total/yesterday
APIService-->>ReactQuery: return stats (+updatedAt)
ReactQuery-->>BlocksComponent: provide data + dataUpdatedAt
BlocksComponent->>BlocksComponent: compute dataUpdatedAt = min(...)
BlocksComponent->>CardContent: render with dataUpdatedAt
Note right of BlocksComponent: if polling enabled
BlocksComponent->>ReactQuery: refetchTotal()
BlocksComponent->>ReactQuery: refetchYesterday()
ReactQuery->>APIService: refetch endpoints
APIService-->>ReactQuery: return fresh stats (+updatedAt)
ReactQuery-->>BlocksComponent: updated data + dataUpdatedAt
BlocksComponent->>CardContent: update age from dataUpdatedAt
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/pages/blocks/index.tsx (3)
264-272: Age doesn't immediately update when data refreshes.When
dataUpdatedAtchanges (after a refetch), the effect clears the old interval and creates a new one, but theagestate won't update until the next 1-second tick. This causes a brief delay where the displayed age is stale.Consider setting the age immediately when
dataUpdatedAtchanges:♻️ Proposed fix
useEffect(() => { + setAge(getAge(new Date(dataUpdatedAt ?? Date.now()))); + const interval = setInterval(() => { setAge(getAge(new Date(dataUpdatedAt ?? Date.now()))); }, 1 * 1000); // 1 sec return () => { clearInterval(interval); }; }, [dataUpdatedAt]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 264 - 272, The age display is stale for up to 1s after dataUpdatedAt changes because the effect only schedules the first update on the next interval tick; modify the effect that uses setInterval (the effect watching dataUpdatedAt) to call setAge(getAge(new Date(dataUpdatedAt ?? Date.now()))) immediately before creating the interval so the age updates synchronously when dataUpdatedAt changes, then continue with the existing interval/clearInterval logic in the same effect.
175-183: Missing dependencies in useEffect.Both
refetchYesterdayandrefetchTotalare used inside the effect but not included in the dependency array. While React Query's refetch functions are stable references, including them follows React best practices and prevents ESLint warnings.♻️ Proposed fix
useEffect(() => { if (blocksInterval) { const intervalId = setInterval(() => { refetchYesterday(); refetchTotal(); }, blocksWatcherInterval); return () => clearInterval(intervalId); } - }, [blocksInterval]); + }, [blocksInterval, refetchYesterday, refetchTotal]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 175 - 183, The effect uses refetchYesterday, refetchTotal and blocksWatcherInterval but only lists blocksInterval in its dependency array; update the useEffect dependency array to include refetchYesterday, refetchTotal and blocksWatcherInterval (in addition to blocksInterval) so the effect re-runs when any of those stable references change and to satisfy React/ESLint expectations for useEffect, keeping the existing interval setup (setInterval/clearInterval) and behavior intact.
254-296: Component defined inside parent causes unnecessary remounts.
CardContentis defined inside theBlockscomponent, which means it's recreated on every render. React treats each recreation as a new component type, causing full remounts that reset internal state (theagetimer) and restart effects. This could cause the timer to flicker or reset unexpectedly during parent re-renders.Consider moving
CardContentoutside theBlockscomponent to make it a stable reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 254 - 296, The CardContent component is currently declared inside the Blocks component causing remounts; to fix, move the CardContent declaration to module scope (outside Blocks) and keep its signature as const CardContent: React.FC<PropsWithChildren<ICard>> = ({ title, headers, values, dataUpdatedAt }) => { ... } so it receives the same props (including dataUpdatedAt and getAge usage) but is a stable component reference; optionally wrap the moved CardContent in React.memo(CardContent) to prevent unnecessary re-renders when parent props don't change.src/services/apiCalls.ts (1)
20-42: Inconsistent error handling in statistics functions.While
blockCallwas correctly refactored to throw on error,yesterdayStatisticsCallandtotalStatisticsCallstill returnres.error(a string) on the error path. This violates the declaredPromise<IBlockStats>return type and prevents errors from propagating to React Query's error state.Consider applying the same pattern used in
blockCallfor consistency:♻️ Proposed fix for consistent error handling
export const yesterdayStatisticsCall = async (): Promise<IBlockStats> => { const res = await api.get({ route: 'block/statistics-by-day/1', }); if (!res.error || res.error === '') { return res.data.block_stats_by_day[0]; - } else { - return res.error; } + + throw new Error(res.error); }; export const totalStatisticsCall = async (): Promise<IBlockStats> => { const res = await api.get({ route: 'block/statistics-total/0', }); if (!res.error || res.error === '') { return res.data.block_stats_total; - } else { - return res.error; } + + throw new Error(res.error); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/apiCalls.ts` around lines 20 - 42, yesterdayStatisticsCall and totalStatisticsCall currently return res.error (a string) on failure which violates Promise<IBlockStats> and prevents errors from propagating; change them to follow the same pattern as blockCall: if res.error is present (res.error && res.error !== '') throw a new Error(res.error) so the promise rejects, otherwise return the correct data (res.data.block_stats_by_day[0] for yesterdayStatisticsCall and res.data.block_stats_total for totalStatisticsCall); reference the functions yesterdayStatisticsCall, totalStatisticsCall and the api.get response object res when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/blocks/index.tsx`:
- Around line 264-272: The age display is stale for up to 1s after dataUpdatedAt
changes because the effect only schedules the first update on the next interval
tick; modify the effect that uses setInterval (the effect watching
dataUpdatedAt) to call setAge(getAge(new Date(dataUpdatedAt ?? Date.now())))
immediately before creating the interval so the age updates synchronously when
dataUpdatedAt changes, then continue with the existing interval/clearInterval
logic in the same effect.
- Around line 175-183: The effect uses refetchYesterday, refetchTotal and
blocksWatcherInterval but only lists blocksInterval in its dependency array;
update the useEffect dependency array to include refetchYesterday, refetchTotal
and blocksWatcherInterval (in addition to blocksInterval) so the effect re-runs
when any of those stable references change and to satisfy React/ESLint
expectations for useEffect, keeping the existing interval setup
(setInterval/clearInterval) and behavior intact.
- Around line 254-296: The CardContent component is currently declared inside
the Blocks component causing remounts; to fix, move the CardContent declaration
to module scope (outside Blocks) and keep its signature as const CardContent:
React.FC<PropsWithChildren<ICard>> = ({ title, headers, values, dataUpdatedAt })
=> { ... } so it receives the same props (including dataUpdatedAt and getAge
usage) but is a stable component reference; optionally wrap the moved
CardContent in React.memo(CardContent) to prevent unnecessary re-renders when
parent props don't change.
In `@src/services/apiCalls.ts`:
- Around line 20-42: yesterdayStatisticsCall and totalStatisticsCall currently
return res.error (a string) on failure which violates Promise<IBlockStats> and
prevents errors from propagating; change them to follow the same pattern as
blockCall: if res.error is present (res.error && res.error !== '') throw a new
Error(res.error) so the promise rejects, otherwise return the correct data
(res.data.block_stats_by_day[0] for yesterdayStatisticsCall and
res.data.block_stats_total for totalStatisticsCall); reference the functions
yesterdayStatisticsCall, totalStatisticsCall and the api.get response object res
when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3c17b8b-1e2f-44f1-bf78-b8cb6d78cfc6
📒 Files selected for processing (3)
src/pages/blocks/index.tsxsrc/services/apiCalls.tssrc/types/blocks.ts
… separation - Fix yesterdayStatisticsCall and totalStatisticsCall to throw on error instead of silently returning error string as IBlockStats (was causing silent display of zeros on API failures) - Normalize error messages with typeof guard for non-string errors - Revert dataUpdatedAt from shared ICard type to local CardContentProps - Track both queries' dataUpdatedAt using Math.min for accurate staleness - Remove silent Date.now() fallback, show empty string before first fetch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/blocks/index.tsx (1)
260-306:⚠️ Potential issue | 🟠 MajorMove
CardContentcomponent andCardContentPropsinterface outside theBlocksfunction.Defining a component inside another component causes it to be recreated on every parent render. React treats each new function reference as a different component type, triggering unmount/remount cycles. This causes:
- Loss of internal state (
age) on every parent re-render- Interval being cleared and recreated unnecessarily
- Potential UI flickering
♻️ Proposed fix: Extract to module scope
Move the interface and component above the
Blockscomponent:+interface CardContentProps extends ICard { + dataUpdatedAt: number; +} + +const CardContent: React.FC<PropsWithChildren<CardContentProps>> = ({ + title, + headers, + values, + dataUpdatedAt, +}) => { + const [age, setAge] = useState( + dataUpdatedAt ? getAge(new Date(dataUpdatedAt)) : '', + ); + + useEffect(() => { + const interval = setInterval(() => { + setAge(dataUpdatedAt ? getAge(new Date(dataUpdatedAt)) : ''); + }, 1 * 1000); + + return () => { + clearInterval(interval); + }; + }, [dataUpdatedAt]); + + return ( + <Card> + <div> + <span> + <strong>{title}</strong> + </span> + <p>{age} ago</p> + </div> + <div> + <span> + <small>{headers[0]}</small> + </span> + <span> + <small>{headers[1]}</small> + </span> + </div> + <div> + <span>{values[0]}</span> + <span>{values[1]}</span> + </div> + </Card> + ); +}; + const Blocks: React.FC<PropsWithChildren<IBlocks>> = () => { // ... remove CardContentProps and CardContent from here🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 260 - 306, The CardContentProps interface and CardContent component are currently declared inside the Blocks function which causes remounts and state/interval loss; extract the interface CardContentProps and the CardContent React.FC (which uses getAge and state/ useEffect for age) to module scope (move them above the Blocks component) and keep the same props (title, headers, values, dataUpdatedAt) so Blocks simply renders <CardContent ... />; ensure the useEffect still depends on dataUpdatedAt and that the component remains in the same file or is exported/imported if moved to a separate module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/blocks/index.tsx`:
- Around line 270-272: The current initialization of age via const [age, setAge]
= useState(dataUpdatedAt ? getAge(new Date(dataUpdatedAt)) : '') can leave age
as an empty string and cause the UI to render " ago" before data loads; change
the initialization to a clear loading/placeholder value (e.g., null or
"Loading...") and update rendering to conditionally show the "ago" suffix only
when age is non-empty (adjust the render logic that uses age), and apply the
same fix to the other age state usage (the similar useState call around getAge)
so neither instance shows a stray " ago" while dataUpdatedAt is falsy.
---
Outside diff comments:
In `@src/pages/blocks/index.tsx`:
- Around line 260-306: The CardContentProps interface and CardContent component
are currently declared inside the Blocks function which causes remounts and
state/interval loss; extract the interface CardContentProps and the CardContent
React.FC (which uses getAge and state/ useEffect for age) to module scope (move
them above the Blocks component) and keep the same props (title, headers,
values, dataUpdatedAt) so Blocks simply renders <CardContent ... />; ensure the
useEffect still depends on dataUpdatedAt and that the component remains in the
same file or is exported/imported if moved to a separate module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31d30b1c-1846-47dd-828e-8ca8ee3ca81d
📒 Files selected for processing (2)
src/pages/blocks/index.tsxsrc/services/apiCalls.ts
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #601 +/- ##
==========================================
- Coverage 0.46% 0.46% -0.01%
==========================================
Files 530 530
Lines 20951 20952 +1
Branches 4982 5287 +305
==========================================
Hits 97 97
+ Misses 20854 20043 -811
- Partials 0 812 +812 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/pages/blocks/index.tsx (2)
181-189: Consider adding refetch functions to the dependency array.While
refetchfunctions from React Query are typically stable references, including them in the dependency array satisfies the exhaustive-deps lint rule and makes the effect's dependencies explicit.♻️ Suggested fix
useEffect(() => { if (blocksInterval) { const intervalId = setInterval(() => { refetchYesterday(); refetchTotal(); }, blocksWatcherInterval); return () => clearInterval(intervalId); } - }, [blocksInterval]); + }, [blocksInterval, refetchYesterday, refetchTotal]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 181 - 189, The effect watching blocksInterval should explicitly include its external dependencies: add refetchYesterday, refetchTotal and blocksWatcherInterval to the useEffect dependency array so the effect re-runs correctly and satisfies exhaustive-deps; update the useEffect that references blocksInterval, refetchYesterday, refetchTotal, and blocksWatcherInterval (and keep the existing clearInterval cleanup) to include those symbols in the dependency list.
260-306: MoveCardContentandCardContentPropsoutside of theBlockscomponent.Defining a component inside another component causes React to create a new function reference on every parent render. This leads to unmounting and remounting all
CardContentinstances, which:
- Resets internal state and restarts intervals unnecessarily
- Causes extra DOM operations affecting performance
While the current behavior still works correctly (due to
dataUpdatedAtprop re-initialization), extracting the component avoids these inefficiencies.♻️ Suggested refactor
Move the interface and component to module scope (before the
Blockscomponent):+interface CardContentProps extends ICard { + dataUpdatedAt: number; +} + +const CardContent: React.FC<PropsWithChildren<CardContentProps>> = ({ + title, + headers, + values, + dataUpdatedAt, +}) => { + const [age, setAge] = useState( + dataUpdatedAt ? getAge(new Date(dataUpdatedAt)) : '', + ); + + useEffect(() => { + const interval = setInterval(() => { + setAge(dataUpdatedAt ? getAge(new Date(dataUpdatedAt)) : ''); + }, 1 * 1000); + + return () => { + clearInterval(interval); + }; + }, [dataUpdatedAt]); + + return ( + <Card> + <div> + <span> + <strong>{title}</strong> + </span> + <p>{age ? `${age} ago` : ''}</p> + </div> + <div> + <span> + <small>{headers[0]}</small> + </span> + <span> + <small>{headers[1]}</small> + </span> + </div> + <div> + <span>{values[0]}</span> + <span>{values[1]}</span> + </div> + </Card> + ); +}; + const Blocks: React.FC<PropsWithChildren<IBlocks>> = () => { // ... remove the internal CardContentProps and CardContent definitions🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/blocks/index.tsx` around lines 260 - 306, The CardContentProps interface and CardContent component are defined inside the Blocks component which causes a new function reference each render and forces remounts; move the CardContentProps interface and the CardContent React.FC (including its useState/useEffect logic that depends on dataUpdatedAt and getAge) to module scope (i.e., outside and above the Blocks component) so CardContent is stable across renders and no longer recreated on each Blocks render; ensure imports (React, useState, useEffect, getAge, Card, ICard, PropsWithChildren) remain available and update any references to CardContent and CardContentProps inside Blocks accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/blocks/index.tsx`:
- Around line 181-189: The effect watching blocksInterval should explicitly
include its external dependencies: add refetchYesterday, refetchTotal and
blocksWatcherInterval to the useEffect dependency array so the effect re-runs
correctly and satisfies exhaustive-deps; update the useEffect that references
blocksInterval, refetchYesterday, refetchTotal, and blocksWatcherInterval (and
keep the existing clearInterval cleanup) to include those symbols in the
dependency list.
- Around line 260-306: The CardContentProps interface and CardContent component
are defined inside the Blocks component which causes a new function reference
each render and forces remounts; move the CardContentProps interface and the
CardContent React.FC (including its useState/useEffect logic that depends on
dataUpdatedAt and getAge) to module scope (i.e., outside and above the Blocks
component) so CardContent is stable across renders and no longer recreated on
each Blocks render; ensure imports (React, useState, useEffect, getAge, Card,
ICard, PropsWithChildren) remain available and update any references to
CardContent and CardContentProps inside Blocks accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7e18600-1119-44dc-93a0-e4c749d10740
📒 Files selected for processing (1)
src/pages/blocks/index.tsx
samufacanha2
approved these changes
Mar 9, 2026
samufacanha2
deleted the
fix/blocks-page-auto-update-timer-and-api-pattern
branch
samufacanha2
added a commit
that referenced
this pull request
Mar 9, 2026
* Merge pull request #593 from klever-io/chore/remove-sc-24h-txs chore: remove sc 24h txs * Merge pull request #602 from klever-io/fix/nft-precision-tx-list fix: nft precision tx list * fix: blocks page stale auto-update, timer drift, and Promise anti-pattern (#601) * fix: blocks page stale auto-update, timer drift, and Promise anti-pattern Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address review findings - consistent error handling, proper type separation - Fix yesterdayStatisticsCall and totalStatisticsCall to throw on error instead of silently returning error string as IBlockStats (was causing silent display of zeros on API failures) - Normalize error messages with typeof guard for non-string errors - Revert dataUpdatedAt from shared ICard type to local CardContentProps - Track both queries' dataUpdatedAt using Math.min for accurate staleness - Remove silent Date.now() fallback, show empty string before first fetch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: prevent rendering bare " ago" before data loads Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: David Ianakiara <david@klever.io> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Three code quality fixes on the blocks page and API calls:
Stale auto-update —
totalStatisticsCallwas never refetched during auto-update intervals. Now bothrefetchYesterday()andrefetchTotal()are called.Timer drift — The "X ago" timer in card headers tracked component mount time, not last data fetch. Now uses React Query's
dataUpdatedAttimestamp which resets on every successful refetch.Promise anti-pattern —
blockCallusedasyncexecutor insidenew Promise, silently swallowing errors. Replaced with standard async/await + throw pattern.Changes
src/pages/blocks/index.tsxrefetch: refetchTotalfromtotalStatisticsCallqueryrefetch→refetchYesterdayfor claritydataUpdatedAtfrom yesterday query, pass toCardContentuptimestate withdataUpdatedAtpropdataUpdatedAtchangessrc/services/apiCalls.tsnew Promise(async (resolve, reject) => { ... })with standardasync/await+throwreturnflow (previouslyresolve()fell through toreject())src/types/blocks.tsdataUpdatedAt?: numbertoICardinterfaceTest plan
Closes #599
🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Bug Fixes