github-starred/cs249r_book
2
0
Fork 0
mirror of https://github.com/harvard-edge/cs249r_book.git synced 2026-07-16 23:24:55 -05:00
Code Issues 276 Packages Projects Releases 26 Wiki Activity

[PR #1913] [MERGED] fix(staffml): cancel StarGate's verify-delay timer on unmount #34642

New Issue
Closed
opened 2026-07-14 19:27:09 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-07-14 19:27:09 -05:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1913
Author: @farhan523
Created: 6/28/2026
Status: ✅ Merged
Merged: 7/10/2026
Merged by: @profvjreddi

Base: dev ← Head: fix/stargate-clear-verify-timeout

📝 Commits (1)

  • 91fe269 fix(staffml): cancel StarGate's verify-delay timer on unmount

📊 Changes

2 files changed (+92 additions, -2 deletions)

View changed files

➕ interviews/staffml/src/__tests__/stargate-timer-cleanup.test.tsx (+74 -0)
📝 interviews/staffml/src/components/StarGate.tsx (+18 -2)

📄 Description

Summary

Both handleStar ("Star on GitHub") and handleAlreadyStarred ("I already starred") fired setTimeout(onVerified, 1500) to let the user see the "Thank you" confirmation panel before the parent dismisses the gate. The returned timer id was never stored, so if the gate unmounted within that 1.5s window — parent route change, programmatic close, or anything that removes StarGate from the tree — the timer kept running and onVerified fired later on an unmounted parent.

Today the only caller (practice/page.tsx) treats a stray onVerified call as a no-op (it just sets showStarGate=false which was already false), so this was a latent leak, not a visible bug. But the contract becomes accidentally correct rather than correct-by-design — a future caller where onVerified mutates state on a stale tree would surface it immediately.

Changes

  • Store the timer id in verifyTimeoutRef (mirrors the focus-timer pattern already in the same file).
  • Add a no-deps cleanup useEffect that clears verifyTimeoutRef.current on unmount.
  • Both handlers assign into the same ref, which also implicitly handles the unlikely back-to-back click case.

Test plan

  • npx vitest run → 129/129 across 23 files passing (was 126; +3 new for unmount-after-Star, unmount-after-Already-starred, and still-mounted-fires-at-1.5s).
  • npx tsc --noEmit → clean.
  • Manual repro (the regression isn't user-visible today; the test is the verification surface).

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1913 **Author:** [@farhan523](https://github.com/farhan523) **Created:** 6/28/2026 **Status:** ✅ Merged **Merged:** 7/10/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/stargate-clear-verify-timeout` --- ### 📝 Commits (1) - [`91fe269`](https://github.com/harvard-edge/cs249r_book/commit/91fe269d91e51de3bdd042da6066df7fae4f7429) fix(staffml): cancel StarGate's verify-delay timer on unmount ### 📊 Changes **2 files changed** (+92 additions, -2 deletions) <details> <summary>View changed files</summary> ➕ `interviews/staffml/src/__tests__/stargate-timer-cleanup.test.tsx` (+74 -0) 📝 `interviews/staffml/src/components/StarGate.tsx` (+18 -2) </details> ### 📄 Description ## Summary Both `handleStar` (\"Star on GitHub\") and `handleAlreadyStarred` (\"I already starred\") fired `setTimeout(onVerified, 1500)` to let the user see the \"Thank you\" confirmation panel before the parent dismisses the gate. The returned timer id was never stored, so if the gate unmounted within that 1.5s window — parent route change, programmatic close, or anything that removes `StarGate` from the tree — the timer kept running and `onVerified` fired later on an unmounted parent. Today the only caller (`practice/page.tsx`) treats a stray `onVerified` call as a no-op (it just sets `showStarGate=false` which was already false), so this was a **latent** leak, not a visible bug. But the contract becomes accidentally correct rather than correct-by-design — a future caller where `onVerified` mutates state on a stale tree would surface it immediately. ### Changes - Store the timer id in `verifyTimeoutRef` (mirrors the focus-timer pattern already in the same file). - Add a no-deps cleanup `useEffect` that clears `verifyTimeoutRef.current` on unmount. - Both handlers assign into the same ref, which also implicitly handles the unlikely back-to-back click case. ## Test plan - [x] `npx vitest run` → 129/129 across 23 files passing (was 126; +3 new for unmount-after-Star, unmount-after-Already-starred, and still-mounted-fires-at-1.5s). - [x] `npx tsc --noEmit` → clean. - [ ] Manual repro (the regression isn't user-visible today; the test is the verification surface). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-07-14 19:27:09 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area: book
area: collabs
area: kits
area: labs
area: socratiq
area: staffml
area: tinytorch
area: tools
area: website
bug
dependencies
format: epub
format: pdf
javascript
link-health
link-rot
priority-high
pull-request
Mirrored from GitHub Pull Request
 staffml
type: bug
type: citation
type: code
type: errata
type: improvement
type: new
type: question
vault-sli
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#34642
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?