fix: resolve 4 bugs in termui - #3618
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe changes fix whitespace-only line parsing, limit hexadecimal entity decoding, log ChangesParsing and UI behavior fixes
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 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.6)packages/ui/src/Form.tsFile contains syntax errors that prevent linting: Line 142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'. 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: 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 `@examples/rss-reader/src/index.tsx`:
- Around line 30-31: Update the entity-decoding logic around codePoint to
extract the complete hexadecimal digit sequence from entity rather than slicing
a fixed range. Reject and return match when the sequence exceeds eight digits,
parse the full sequence, and also reject values above 0x10FFFF; otherwise
preserve the existing String.fromCodePoint conversion.
In `@packages/ui/src/Form.ts`:
- Around line 141-142: Remove the orphaned catch handler after the Form class
and ensure Promise.all rejection handling is attached within the submit()
method’s validation flow, or remove it if unused, so Form.ts parses correctly.
🪄 Autofix
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: 4412a857-b8dc-4dcb-b908-afcf912397f3
📒 Files selected for processing (4)
examples/chat-app/src/index.tsxexamples/rss-reader/src/index.tsxpackages/ui/src/Form.tspackages/ui/src/Switch.ts
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Locate file:\n'
fd -a 'index\.tsx$' . | sed 's#^\./##' | rg 'rss-reader/src/index\.tsx$|examples/rss-reader/src/index\.tsx$' || true
printf '\nRelevant file excerpt:\n'
sed -n '1,80p' examples/rss-reader/src/index.tsx
printf '\nOccurrences of parseInt/String.fromCodePoint/pattern:\n'
rg -n "number|Number\.parseInt|String\.fromCodePoint|fromCodePoint|\&`#x`|codePoint" examples/rss-reader/src/index.tsx || true
printf '\nJS behavioral probe for reviewed behavior:\n'
node - <<'JS'
const match = '&`#x110000`;';
const used = match.slice(2, 10);
console.log({ full: match, used });
console.log({ parseIntFinite: Number.isFinite(Number.parseInt(used, 16)) });
console.log({ fromCodePointThrows: Object.getOwnPropertyDescriptor(Object.getOwnPropertyDescriptor(Object.getPrototypeOf(String), 'fromCodePoint'), 'length').value });
try {
String.fromCodePoint(Number.parseInt(used, 16));
console.log('fromCodePoint did not throw');
} catch (e) {
console.log({ fromCodePointError: e.message });
}
console.log('0x1FFFF:', Number.parseInt('FFFFFFFF', 16), Number.isFinite(Number.parseInt('FFFFFFFF', 16)));
try {
String.fromCodePoint(0xFFFFFFFF);
} catch (e) {
console.log('0xFFFFFFFF error:', e.message);
}
JSRepository: Karanjot786/TermUI
Length of output: 4149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Locate file:\n'
fd -a 'index\.tsx$' . | sed 's#^\./##' | rg 'rss-reader/src/index\.tsx$|examples/rss-reader/src/index\.tsx$' || true
printf '\nRelevant file excerpt:\n'
sed -n '1,80p' examples/rss-reader/src/index.tsx
printf '\nOccurrences of parseInt/String.fromCodePoint/pattern:\n'
rg -n "number|Number\.parseInt|String\.fromCodePoint|fromCodePoint|\&`#x`|codePoint" examples/rss-reader/src/index.tsx || true
printf '\nJS behavioral probe for reviewed behavior:\n'
node - <<'JS'
const match = '&`#x110000`;';
const used = match.slice(2, 10);
console.log({ full: match, used });
console.log({ parseIntFinite: Number.isFinite(Number.parseInt(used, 16)) });
try {
String.fromCodePoint(Number.parseInt(used, 16));
console.log('fromCodePoint did not throw');
} catch (e) {
console.log({ fromCodePointError: e.message });
}
console.log('0x1FFFF:', Number.parseInt('FFFFFFFF', 16), Number.isFinite(Number.parseInt('FFFFFFFF', 16)));
try {
String.fromCodePoint(0xFFFFFFFF);
} catch (e) {
console.log('0xFFFFFFFF error:', e.message);
}
JSRepository: Karanjot786/TermUI
Length of output: 3475
Reject hexadecimal entities that exceed eight digits or the Unicode code-point limit.
entity.slice(2, 10) includes characters after the hex digits and ignores the full matched entity length. Malformed entities such as � do not pass Number.isFinite, but entities longer than eight hex digits can be decoded as truncated values, and non-surrogate values above 0xFFFF below 0x10FFFF are also valid characters.
Extract the actual hex digits from the matched entity, return match when that sequence exceeds eight digits, parse the full sequence, and return match when the parsed value exceeds 0x10FFFF.
Proposed fix
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
- return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+ const digits = entity.slice(2, -1);
+ if (digits.length > 8) return match;
+ const codePoint = Number.parseInt(digits, 16);
+ return Number.isFinite(codePoint) && codePoint <= 0x10FFFF
+ ? String.fromCodePoint(codePoint)
+ : match;📝 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); | |
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; | |
| const digits = entity.slice(2, -1); | |
| if (digits.length > 8) return match; | |
| const codePoint = Number.parseInt(digits, 16); | |
| return Number.isFinite(codePoint) && codePoint <= 0x10FFFF | |
| ? String.fromCodePoint(codePoint) | |
| : match; |
🤖 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` around lines 30 - 31, Update the
entity-decoding logic around codePoint to extract the complete hexadecimal digit
sequence from entity rather than slicing a fixed range. Reject and return match
when the sequence exceeds eight digits, parse the full sequence, and also reject
values above 0x10FFFF; otherwise preserve the existing String.fromCodePoint
conversion.
|
|
||
| .catch(err => console.error("Promise.all failed:", err)); No newline at end of file |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '75,145p' packages/ui/src/Form.ts
if rg -n '^\s*\.catch\s*\(' packages/ui/src/Form.ts; then
echo "Found a standalone catch chain."
exit 1
fi
if ! rg -n 'Promise\.all\(validationPromises\).*\.catch|try\s*\{' packages/ui/src/Form.ts; then
echo "No valid rejection-handling construct found."
exit 1
fiRepository: Karanjot786/TermUI
Length of output: 3039
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Form.ts outline =="
ast-grep outline packages/ui/src/Form.ts || true
echo "== Full Form.ts catch/block context =="
sed -n '1,180p' packages/ui/src/Form.ts
echo "== Standalone catch parse probe =="
node - <<'JS'
const source = `
class X { async m() { await 1; } }
Promise.all([1]).catch(e => console.log(e))
`;
try {
const f = new Function(source);
console.log("Node parsed source successfully");
f();
} catch (err) {
console.log("Parse/runtime error:", err.message);
}
JSRepository: Karanjot786/TermUI
Length of output: 7382
Remove the orphaned Promise.all(...).catch(...) outside the class.
This .catch appears after the closing class Form }, not on the Promise.all(...) expression, so packages/ui/src/Form.ts fails to parse. Move the rejection handling into the submit() method where validation runs, or remove the dead handler entirely.
🧰 Tools
🪛 Biome (2.5.6)
[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.
(parse)
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 142-142: @termuijs/ui build failed during tsup/esbuild. Unexpected '.' in standalone '.catch(err => console.error("Promise.all failed:", err));', causing TypeScript syntax errors (TS1128, TS1005, TS2304). Command failed: bun run build (exit code 1).
🪛 GitHub Actions: CI / build-and-test
[error] 142-142: @termuijs/ui build (tsup/esbuild) failed: Unexpected "." at the leading .catch(...) expression. TypeScript also reports TS1128, TS1005, and TS2304 syntax errors. Command: bun run build.
🤖 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/Form.ts` around lines 141 - 142, Remove the orphaned catch
handler after the Form class and ensure Promise.all rejection handling is
attached within the submit() method’s validation flow, or remove it if unused,
so Form.ts parses correctly.
Source: Linters/SAST tools
1 participant
Description
This PR fixes real bugs found in the codebase:
trim()to''misses whitespace-only input;.trim().length === 0is explicit.Promise.all: an unhandled rejection in any input promise previously crashed silently.Number.EPSILONtoMath.round: prevents floating-point drift (e.g.1.005 * 100rounding to 100 instead of 101).parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3617
Summary by CodeRabbit