fix: resolve 4 bugs in termui - #3446
Conversation
github-actions
Bot
added
area:examples
📝 WalkthroughWalkthroughFour isolated fixes update streaming interval cleanup, form shortcut detection, RSS hexadecimal entity parsing, and switch knob rounding. ChangesStreaming timer management
Form shortcut detection
RSS entity parsing
Switch positioning
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/ui/src/Switch.ts (1)
117-117: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the rounding thresholds.
Line 117 changes knob placement near the
0.25and0.75progress boundaries. The existingpackages/ui/src/Switch.test.tstest checks only rendering bounds. Add focused tests for values just below and at both thresholds to verify thatNumber.EPSILONcorrects floating-point drift without changing behavior away from those boundaries.🤖 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/ui/src/Switch.ts` at line 117, Add focused regression tests in Switch.test.ts for knob placement around the 0.25 and 0.75 animation-progress thresholds, covering values just below and exactly at each boundary. Verify Number.EPSILON preserves the expected rounded knob position at the thresholds while leaving behavior unchanged for progress values away from them.
🤖 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 `@examples/ai-streaming/src/index.tsx`:
- Around line 43-45: Update the interval handling around _streamingText.tick to
provide an explicit TypeScript declaration for window.__interval, either by
augmenting the Window interface or by using an explicitly typed interval handle
with an explanatory comment. Preserve the existing clearInterval and setInterval
behavior while ensuring the example passes strict DOM TypeScript checks.
In `@examples/forms-and-validation/src/index.tsx`:
- Line 125: In the keyboard event condition, update the `event.ctrl` negation in
the `if` statement so the `!` operator precedes the property access, producing
valid TypeScript syntax while preserving the existing key check.
In `@examples/rss-reader/src/index.tsx`:
- Line 30: Update the entity-decoding logic around codePoint so hexadecimal
entities longer than eight digits are rejected before parsing, or otherwise
parsed without silently truncating digits. Preserve valid entities such as
&`#x000000041`; by ensuring all captured hexadecimal digits are handled according
to the intended length limit.
---
Nitpick comments:
In `@packages/ui/src/Switch.ts`:
- Line 117: Add focused regression tests in Switch.test.ts for knob placement
around the 0.25 and 0.75 animation-progress thresholds, covering values just
below and exactly at each boundary. Verify Number.EPSILON preserves the expected
rounded knob position at the thresholds while leaving behavior unchanged for
progress values away from them.
🪄 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: 6df8cc6c-2271-475d-b9ff-9e32a7eec773
📒 Files selected for processing (4)
examples/ai-streaming/src/index.tsxexamples/forms-and-validation/src/index.tsxexamples/rss-reader/src/index.tsxpackages/ui/src/Switch.ts
| clearInterval(window.__interval); window.__interval = setInterval(() => { | ||
| this._streamingText.tick(); | ||
| }, 50); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect: a Window augmentation or another declaration for __interval.
rg -n -C 3 '(__interval|interface Window|declare global)' --glob '*.ts' --glob '*.tsx'Repository: Karanjot786/TermUI
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked TypeScript/TSX files containing __interval:"
git ls-files '*.ts' '*.tsx' | xargs grep -n '__interval' || true
echo
echo "Tracked TypeScript config files:"
git ls-files '*tsconfig*.json' 'tsconfig*.json' 'package.json' 'vite.config.*' 'astro.config.*'
echo
echo "Relevant file outline:"
if [ -f examples/ai-streaming/src/index.tsx ]; then
ast-grep outline examples/ai-streaming/src/index.tsx || true
echo
sed -n '1,120p' examples/ai-streaming/src/index.tsx | nl -ba
fi
echo
echo "Other explicit window property declarations/augmentations:"
git ls-files '*.ts' '*.tsx' '*.d.ts' | xargs grep -nE 'interface\s+Window|declare\s+global|declare\s+namespace\s+Window|\.(__interval)|window\.__interval|ReturnType<.*setInterval|number' || trueRepository: Karanjot786/TermUI
Length of output: 2606
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "tsconfig files for ai-streaming/example base:"
while IFS= read -r f; do
echo "--- $f"
sed -n '1,220p' "$f"
done < <(git ls-files | grep -E '(^examples/ai-streaming/.*tsconfig\.json$|(^tsconfig\.base\.json$|(^package\.json$))')
echo
echo "All __interval occurrences under git:"
git ls-files | grep -E '\.(ts|tsx|d\.ts)$' | xargs grep -n -- '__interval' || true
echo
echo "Window type augmentations/decorations under git:"
git ls-files '*.ts' '*.tsx' '*.d.ts' | xargs grep -n -- -E 'interface\s+Window|declare\s+global|declare\s+namespace\s+Window|window\.__|__interval|setInterval' || true
echo
echo "TypeScript availability and toolchain:"
command -v npx || true
command -v tsc || true
node -e "try { console.log('node:', process.version); } catch (e) { console.log(e.message); }"
if command -v tsc >/dev/null 2>&1; then
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT
cat > "$tmp/check.ts" <<'TS'
function tick() {
clearInterval(window.__interval as unknown as number | undefined);
window.__interval = setInterval(() => {
console.log('tick');
}, 50);
}
function main() {
tick();
}
TS
echo "--- tsc version"
tsc --version
echo "--- tsc with defaults"
tsc "$tmp/check.ts" --strict --noEmit || true
else
echo "tsc not available in PATH"
fiRepository: Karanjot786/TermUI
Length of output: 278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files under examples/ai-streaming tracked by git:"
git ls-files examples/ai-streaming | sed -n '1,120p'
echo
echo "Candidate tsconfig/package files:"
git ls-files | grep -E '(^|/)(tsconfig.*\.json|package\.json)$' | sed -n '1,160p'
echo
echo "Relevant ai-streaming tsconfig/package if present:"
for f in examples/ai-streaming/tsconfig.json examples/ai-streaming/tsconfig.*.json examples/ai-streaming/package.json tsconfig.base.json package.json; do
if [ -f "$f" ]; then
echo "--- $f"
sed -n '1,220p' "$f"
fi
done
echo
echo "All __interval occurrences in tracked source:"
while IFS= read -r f; do
grep -n -- '__interval' "$f" || true
done < <(git ls-files '*.ts' '*.tsx' '*.d.ts')
echo
echo "TypeScript available:"
command -v tsc || true
tsc --version 2>/dev/null || true
if command -v tsc >/dev/null 2>&1; then
tmp="$(mktemp -d)"
cat > "$tmp/check.ts" <<'PY'
function tick() {
clearInterval(window.__interval as unknown as number | undefined);
window.__interval = setInterval(() => {
console.log('tick');
}, 50);
}
function main() {
tick();
}
PY
echo "--- tsc strict/noEmit test"
tsc "$tmp/check.ts" --strict --noEmit --lib ES2020,DOM || true
fiRepository: Karanjot786/TermUI
Length of output: 6672
Avoid using window.__interval without a type declaration.
Under strict TypeScript checks, window.__interval has no declaration in the repo, so this example can fail npx -y tsc --strict --lib ES2022,DOM unless the type definitions are intentionally relying on the run-time host. Add a Window augmentation or type the handle explicitly with a comment if the global is intentional.
🤖 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 `@examples/ai-streaming/src/index.tsx` around lines 43 - 45, Update the
interval handling around _streamingText.tick to provide an explicit TypeScript
declaration for window.__interval, either by augmenting the Window interface or
by using an explicitly typed interval handle with an explanatory comment.
Preserve the existing clearInterval and setInterval behavior while ensuring the
example passes strict DOM TypeScript checks.
Source: Coding guidelines
| } | ||
|
|
||
| if (event.key === 'c' && event.ctrl === false) { | ||
| if (event.key === 'c' && event.ctrl !) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix the invalid TypeScript condition.
event.ctrl ! is invalid TypeScript syntax and prevents compilation. Move ! before event.ctrl.
Proposed fix
- if (event.key === 'c' && event.ctrl !) {
+ if (event.key === 'c' && !event.ctrl) {📝 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.
| if (event.key === 'c' && event.ctrl !) { | |
| if (event.key === 'c' && !event.ctrl) { |
🤖 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 `@examples/forms-and-validation/src/index.tsx` at line 125, In the keyboard
event condition, update the `event.ctrl` negation in the `if` statement so the
`!` operator precedes the property access, producing valid TypeScript syntax
while preserving the existing key check.
| return value.replace(/&(#x?[0-9a-fA-F]+|[a-zA-Z]+);/g, (match, entity: string) => { | ||
| if (entity.startsWith('#x')) { | ||
| const codePoint = Number.parseInt(entity.slice(2), 16); | ||
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject overlong hexadecimal entities instead of truncating them.
The regular expression captures every hexadecimal digit, but entity.slice(2, 10) silently drops digits after the eighth. For example, A becomes 0x00000004 instead of 0x41, which corrupts RSS text.
If eight digits is the intended limit, reject longer entities before parsing.
Proposed fix
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
+ const digits = entity.slice(2);
+ if (digits.length > 8) {
+ return match;
+ }
+ const codePoint = Number.parseInt(digits, 16);📝 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.
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | |
| const digits = entity.slice(2); | |
| if (digits.length > 8) { | |
| return match; | |
| } | |
| const codePoint = Number.parseInt(digits, 16); |
🤖 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 `@examples/rss-reader/src/index.tsx` at line 30, Update the entity-decoding
logic around codePoint so hexadecimal entities longer than eight digits are
rejected before parsing, or otherwise parsed without silently truncating digits.
Preserve valid entities such as &`#x000000041`; by ensuring all captured
hexadecimal digits are handled according to the intended length limit.
1 participant
Description
This PR fixes real bugs found in the codebase:
x === trueis equivalent tox(andx === falseto!x), and shorter to read.parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3445
Summary by CodeRabbit