Fix/terminal debounce#711
Merged
Karanjot786 merged 1 commit intoJun 5, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTerminal class now accepts a ChangesTerminal Resize Debouncing and Deduplication
Sequence DiagramsequenceDiagram
participant Listener as resize listener
participant Timer as debounce timer
participant Handler as resize handler
Listener->>Listener: update _cols/_rows immediately
Listener->>Timer: clear prior timer
Listener->>Timer: schedule dispatch after debounce ms
Listener->>Listener: (rapid resize received)
Listener->>Listener: update _cols/_rows
Listener->>Timer: clear and reschedule
Timer->>Handler: dispatch with final dimensions
Handler->>Handler: cache dispatched (cols, rows)
Listener->>Listener: (identical resize received)
Listener->>Listener: update _cols/_rows
Listener->>Timer: clear and reschedule
Timer->>Handler: skip dispatch (dims unchanged from last dispatch)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
type:devops
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/terminal/Terminal.test.ts`:
- Line 9: The test's fakeStdout variable declares write: any which violates the
"no any without inline comment" rule; update the declaration in Terminal.test.ts
(the fakeStdout variable) by either replacing `any` with the correct function
type for the stream write (e.g., a mock signature) or, if that isn't practical,
add a brief inline comment after `any` explaining why `any` is necessary (for
example: /* uses a simplified mock of stream.write signature for tests */) so
the linter rule is satisfied.
- Line 22: The test uses a double type assertion "fakeStdout as unknown as
NodeJS.WriteStream" without explanation; add an inline comment next to this
assertion in Terminal.test.ts explaining why the assertion is required (e.g.,
fakeStdout is a simplified mock that doesn’t implement the full
NodeJS.WriteStream interface or TS types are incompatible for the test),
referencing the fakeStdout variable and the NodeJS.WriteStream cast so future
readers know the rationale and can safely refactor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 713a943e-f6e9-4cee-8495-7bb69f58710b
📒 Files selected for processing (6)
packages/core/src/terminal/Terminal.test.tspackages/core/src/terminal/Terminal.tspackages/dev-server/src/server.test.tspackages/jsx/src/index.tspackages/motion/src/index.tspackages/motion/src/sequence.ts
💤 Files with no reviewable changes (2)
- packages/dev-server/src/server.test.ts
- packages/motion/src/sequence.ts
|
|
||
| describe('Resize Debouncing & Deduplication', () => { | ||
| let term: Terminal; | ||
| let fakeStdout: EventEmitter & { columns: number; rows: number; write: any }; |
There was a problem hiding this comment.
Add inline comment for any type.
The write: any property violates the TypeScript guideline requiring an inline comment explaining why any is necessary. As per coding guidelines, "No any without an inline comment explaining why."
📝 Proposed fix
- let fakeStdout: EventEmitter & { columns: number; rows: number; write: any };
+ let fakeStdout: EventEmitter & { columns: number; rows: number; write: any /* mock fn type incompatible with WriteStream.write */ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let fakeStdout: EventEmitter & { columns: number; rows: number; write: any }; | |
| let fakeStdout: EventEmitter & { columns: number; rows: number; write: any /* mock fn type incompatible with WriteStream.write */ }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/terminal/Terminal.test.ts` at line 9, The test's fakeStdout
variable declares write: any which violates the "no any without inline comment"
rule; update the declaration in Terminal.test.ts (the fakeStdout variable) by
either replacing `any` with the correct function type for the stream write
(e.g., a mock signature) or, if that isn't practical, add a brief inline comment
after `any` explaining why `any` is necessary (for example: /* uses a simplified
mock of stream.write signature for tests */) so the linter rule is satisfied.
Karanjot786
added
gssoc:approved
Karanjot786
force-pushed
the
fix/terminal-debounce
branch
from
589fd7b to
d759b7b
Compare
Adds resizeDebounceMs option (default 16ms) to Terminal. Rapid SIGWINCH signals are coalesced into a single resize dispatch with dedup check to avoid redundant handler calls when dimensions haven't changed.
Karanjot786
force-pushed
the
fix/terminal-debounce
branch
from
d759b7b to
b79b15a
Compare
Owner
|
Thank you so much for your contribution, @abhijnyan-codes! Your PR #711 added resize debouncing to Terminal. Rapid SIGWINCH signals now coalesce into a single dispatch, so resize handlers fire once instead of dozens of times. Keep contributing. We appreciate your work. |
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.
Description
This PR implements debouncing and deduplication for terminal resize events (SIGWINCH) in the
Terminaladapter. It adds a configurableresizeDebounceMs(default 16ms) to prevent rapid, redundant resize dispatches while ensuring the final, accurate dimensions are captured. It also guarantees that timers are properly cleared duringrestore()to prevent memory leaks.Related Issue
Closes #[ISSUE_NUMBER]
Which package(s)?
@termuijs/core
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/8a07df5d-1e10-436c-b2a5-790d42085965Screenshots / Recordings (UI changes)
N/A - This is a core event logic change without visual layout modifications.
Notes for the Reviewer
packages/coreper the issue specifications.vi.useFakeTimers()and a simulatedstdoutstream to safely verify debounce timing, deduplication, and cleanup without mutating the global process.Summary by CodeRabbit
New Features
resizeDebounceMsoption to control terminal resize debouncing.Tests