fix(react): stop impure Date calls during render + 2 lint cleanups - #8
Conversation
Two real render-purity bugs: EstimatePdf computed the estimate # from Date.now() during render (so it changed on every re-render) and the date likewise; ExpirationAlerts read Date.now() in render. Both now stamp once at mount (useMemo / lazy useState). Also clears a dead eslint-disable in ErrorBoundary and a // comment-textnode in EstimateResults. Lint warnings 22 -> 18 (the rest are the localStorage-hydration set-state-in-effect pattern, left for a deliberate burndown). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughTwo components ( ChangesStable Timestamps and Minor Fixes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe5bb30082
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function ExpirationAlerts({ documents }: ExpirationAlertsProps) { | ||
| const now = Date.now(); | ||
| // Captured once at mount — Date.now() in render is impure. | ||
| const [now] = useState(() => Date.now()); |
There was a problem hiding this comment.
When the vault page stays open across a threshold change, this frozen now keeps all bucket decisions tied to the initial mount time. app/vault/page.tsx keeps ExpirationAlerts mounted for the page lifetime, so a document that was 31 days out at load will not enter the <30 days alert on later refresh/rerender, and an 8-day document can remain in the warning bucket after it crosses 7 days or expires; the per-card status still recomputes from Date.now(), so the banner can disagree with the cards. Track current time with a refresh/remount trigger or derive it when recomputing buckets instead of storing it once.
Useful? React with 👍 / 👎.
1 participant
Burns down the high-confidence subset of the flagged lint warnings (the rest are deliberately left — see below).
Real bugs fixed
EstimatePdfcomputedEST-${Date.now()...}during render, so the estimate number changed on every re-render (and the date was the same impurity class). Both now stamp once at mount viauseMemo.ExpirationAlertsreadDate.now()in render → captured once via lazyuseState.Cleanups
eslint-disable no-consoleinErrorBoundary(the rule isn't enabled).// Total Bid EstimateJSX text node in braces (it's intentional visible cyberpunk-style text, not a stray comment).Verification
tscclean · 74 tests · build OK.Deliberately NOT fixed
The 18 remaining warnings are all
react-hooks/set-state-in-effect— the standard "hydrate from localStorage after mount" pattern (you can't readlocalStorageduring SSR). "Fixing" them meansuseSyncExternalStoreboilerplate at ~14 sites with real regression risk; better as a focused burndown than churned blind. One (document-card) is a lint false-positive (it selects an existing icon, doesn't create a component).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores