fix: resolve 4 bugs in termui - #3629
Conversation
github-actions
Bot
added
type:bug
📝 WalkthroughWalkthroughThe changes apply four targeted reliability fixes across examples and UI form handling. They bound hexadecimal entity parsing, manage weather refresh intervals, specify decimal keyboard parsing, and log ChangesReliability fixes
Estimated code review effort: 2 (Simple) | ~10 minutes 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 reject
parsed hexadecimal values outside the valid Unicode range, specifically values
greater than 0x10FFFF, before calling String.fromCodePoint; preserve the
existing fallback to match for invalid values.
In `@packages/ui/src/Form.ts`:
- Around line 141-142: Attach the rejection handler directly to the
Promise.all(validationPromises) chain in Form.ts instead of leaving the
standalone .catch expression. In that handler, reset _isValidating, call
markDirty(), and rethrow the error so rejection prevents _onSubmit from running.
🪄 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: 3600c78d-6610-4e0f-8166-cc7d23b496f2
📒 Files selected for processing (4)
examples/rss-reader/src/index.tsxexamples/weather/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/Form.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
rg -n -C 4 'entity\.slice\(2, 10\)|String\.fromCodePoint|Number\.parseInt' \
examples/rss-reader/src/index.tsx
node - <<'NODE'
const codePoint = Number.parseInt('110000', 16);
try {
String.fromCodePoint(codePoint);
throw new Error('Expected String.fromCodePoint to reject 0x110000');
} catch (error) {
if (!(error instanceof RangeError)) throw error;
}
NODERepository: Karanjot786/TermUI
Length of output: 728
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'NODE'
for (const [name, input] of [['hex 0x110000', '110000'], ['hex 0x20000', '20000'], ['hex 0x10FFFF', '10FFFF'], ['hex truncated leading', '0000000010FFFF']]) {
try {
const codePoint = Number.parseInt(input, 16);
const output = Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : 'skip';
console.log(name, 'parseInt=', codePoint.toString(16), 'output=', output?.codePointAt(0)?.toString(16) || output, 'isFinite=', Number.isFinite(codePoint));
} catch (error) {
console.log(name, 'throws', error.message);
}
}
for (const input of ['110000', '11ffff', '1100000']) {
try {
Number.parseInt(input, 16) + String.fromCodePoint(Number.parseInt(input, 16));
} catch (error) {
console.log('String.fromCodePoint rejects', input, error.name, error.message);
}
}
NODE
git diff -- examples/rss-reader/src/index.tsx | sed -n '1,120p' || trueRepository: Karanjot786/TermUI
Length of output: 613
Validate hexadecimal code points before decoding.
entity.slice(2, 10) accepts finite values above 0x10FFFF, so entities like � throw RangeError from String.fromCodePoint. Parse the full hexadecimal value and reject values larger than 0x10FFFF.
Proposed fix
- const codePoint = Number.parseInt(entity.slice(2, 10), 16);
- return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match;
+ const codePoint = Number.parseInt(entity.slice(2), 16);
+ return Number.isInteger(codePoint) && codePoint >= 0 && 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 codePoint = Number.parseInt(entity.slice(2), 16); | |
| return Number.isInteger(codePoint) && codePoint >= 0 && 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 reject parsed hexadecimal values outside the
valid Unicode range, specifically values greater than 0x10FFFF, before calling
String.fromCodePoint; preserve the existing fallback to match for invalid
values.
|
|
||
| .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
npx biome check packages/ui/src/Form.tsRepository: Karanjot786/TermUI
Length of output: 2385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'TypeScript availability:\n'
node - <<'JS'
try {
const ts = require('typescript');
console.log(ts.version);
} catch (e) {
console.log('typescript-not-available:', e.message);
}
JS
printf '\nForm.ts relevant lines:\n'
sed -n '60,150p' packages/ui/src/Form.ts | nl -ba -v60
printf '\nSearch for console.error and validationPromises in packages/ui/src/Form.ts:\n'
rg -n 'console\.error|Promise\.all|validationPromises|_isValidating|markDirty\(' packages/ui/src/Form.ts
printf '\nIf TypeScript is available, probe the exact snippet parse behavior:\n'
node - <<'JS'
try {
const ts = require('typescript');
const src = `async function f() {
const results = await Promise.all([]);
}
.catch(err => console.error("Promise.all failed:", err));
`;
const out = ts.transpileModule(src, { compilerOptions: { target: ts.ScriptTarget.ES2020 } });
console.log(JSON.stringify({ diagnostics: out.diagnostics.map(d => `${d.code} ${d.messageText}`), outputText: out.outputText }, null, 2));
} catch (e) {
console.log(String(e));
}
JSRepository: Karanjot786/TermUI
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Form.ts relevant lines:\n'
sed -n '60,155p' packages/ui/src/Form.ts | cat -n
printf '\nSearch for console.error and validationPromises in packages/ui/src/Form.ts:\n'
rg -n 'console\.error|Promise\.all|validationPromises|_isValidating|markDirty\(' packages/ui/src/Form.ts || true
printf '\nCheck installed Biome and language config availability:\n'
node - <<'JS'
try {
const fs = require('node:fs');
for (const f of ['.biome.json', 'biome.json', '.biome.jsonc', 'biome.jsonc']) {
if (!fs.existsSync(f)) continue;
console.log(`${f}: ${fs.readFileSync(f, 'utf8').trim()}`);
}
console.log('package scripts:', JSON.parse(fs.readFileSync('package.json','utf8')).scripts || {});
} catch (e) {
console.log(String(e));
}
JSRepository: Karanjot786/TermUI
Length of output: 5758
Attach rejection handling to Promise.all.
The .catch(...) at packages/ui/src/Form.ts:142 is a standalone expression, so packages/ui/src/Form.ts cannot compile. Move the handler onto Promise.all(validationPromises), reset _isValidating, call markDirty(), and rethrow it before _onSubmit can run.
🧰 Tools
🪛 Biome (2.5.6)
[error] 142-142: Expected a statement but instead found '.catch(err => console.error("Promise.all failed:", err))'.
(parse)
🤖 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, Attach the rejection handler
directly to the Promise.all(validationPromises) chain in Form.ts instead of
leaving the standalone .catch expression. In that handler, reset _isValidating,
call markDirty(), and rethrow the error so rejection prevents _onSubmit from
running.
Source: Linters/SAST tools
1 participant
Description
This PR fixes real bugs found in the codebase:
parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Promise.all: an unhandled rejection in any input promise previously crashed silently.parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3628
Summary by CodeRabbit