fix: resolve 4 bugs in termui - #3517
Conversation
github-actions
Bot
added
area:examples
📝 WalkthroughWalkthroughThe PR updates percentage rounding, tab-key parsing, child-process error handling, and dependency sorting. The reload error-handling change contains an incomplete catch callback that causes a syntax error. ChangesCorrectness fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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/dev-server/src/server.tsFile contains syntax errors that prevent linting: Line 383: Expected a parenthesis '(' but instead found '=>'. 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 `@packages/dev-server/src/server.ts`:
- Line 383: Fix the malformed catch callback on exitedPromise by adding a valid
rejection parameter and pass it to console.error, preserving the existing await
and error-handling flow.
In `@scripts/build-registry.ts`:
- Line 47: Update the dependency sorting in collectDeps to use a string
comparator for identifiers rather than numeric subtraction, ensuring
alphabetical ordering such as `@termuijs/core`, `@termuijs/motion`, and
`@termuijs/widgets` while preserving deduplication.
🪄 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: 45270f72-a220-46ea-b5c0-0574abff90fe
📒 Files selected for processing (4)
examples/pomodoro-timer/src/index.tsxexamples/showcase/src/index.tsxpackages/dev-server/src/server.tsscripts/build-registry.ts
| this._killChild(); | ||
|
|
||
| await exitedPromise.catch(() => {}); | ||
| await exitedPromise.catch( => console.error()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix the malformed .catch callback.
Line 383 is invalid TypeScript because the arrow function has no parameter list. This prevents packages/dev-server/src/server.ts from parsing. Pass the rejection value to console.error:
Proposed fix
- await exitedPromise.catch( => console.error());
+ await exitedPromise.catch((error) => console.error(error));Static analysis confirms the parser error at Line 383.
📝 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.
| await exitedPromise.catch( => console.error()); | |
| await exitedPromise.catch((error) => console.error(error)); |
🧰 Tools
🪛 Biome (2.5.6)
[error] 383-383: Expected a parenthesis '(' but instead found '=>'.
(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/dev-server/src/server.ts` at line 383, Fix the malformed catch
callback on exitedPromise by adding a valid rejection parameter and pass it to
console.error, preserving the existing await and error-handling flow.
Source: Linters/SAST tools
| let m: RegExpExecArray | null; | ||
| while ((m = re.exec(content)) !== null) deps.add(m[1]!); | ||
| return [...deps].sort(); | ||
| return [...deps].sort((a, b) => a - b); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## file list"
git ls-files | rg '(^|/)build-registry(\.test)?\.ts$|scripts/build-registry' || true
echo "## scripts/build-registry.ts outline"
ast-grep outline scripts/build-registry.ts --view compact || true
echo "## scripts/build-registry.ts relevant lines"
sed -n '1,90p' scripts/build-registry.ts
echo "## tests around registry sorting"
sed -n '100,155p' scripts/build-registry.test.ts
echo "## JS runtime behavior for array of strings subtracting"
node - <<'JS'
const deps = ['zebra', 'apple', 'Mango', 'banana'];
console.log(JSON.stringify([
deps.map(v => {
const values = [];
const result = [...deps].sort((a, b) => (values.push([JSON.stringify([a,b]), a-b]), [].sort((a, b) => a - b)(v)).flat())
return undefined;
})
)));
console.log(JSON.stringify({
directSubtractionDemo: deps.map((a,i) => a + ' - ' + deps[i] + ' = ' + (a - deps[i])),
sortBefore: JSON.stringify(deps),
sortMinusComparator: JSON.stringify([...deps].sort((a, b) => a - b)),
naturalSort: JSON.stringify([...deps].sort()),
}))
JSRepository: Karanjot786/TermUI
Length of output: 7130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## runtime behavior for string comparator and JS/TC semantics"
node - <<'JS'
const src = [
`import { Widget } from '`@termuijs/widgets`';`,
`import { Screen } from '`@termuijs/core`';`,
`import { timerPoolSubscribe } from '`@termuijs/motion`';`,
`import { Widget as W2 } from '`@termuijs/widgets`';`,
].join('\n');
const re = /from\s+['"](`@termuijs`\/[a-z-]+)['"]/g;
const deps = new Set();
let m;
while ((m = re.exec(src)) !== null) deps.add(m[1]);
const list = [...deps];
console.log(JSON.stringify({
directSubtractionDemo: {
a: JSON.stringify('`@termuijs/widgets` - `@termuijs/core` = ' + ('`@termuijs/widgets`' - '`@termuijs/core`')),
comparison: JSON.stringify('`@termuijs/motion`' < '`@termuijs/widgets`')
},
sortWithSubtractionComparator: JSON.stringify([...list].sort((a, b) => a - b)),
defaultStringSort: JSON.stringify([...list].sort()),
expected: JSON.stringify(['`@termuijs/core`', '`@termuijs/motion`', '`@termuijs/widgets`']),
inputOrder: JSON.stringify(list)
}));
JSRepository: Karanjot786/TermUI
Length of output: 657
Use a string comparator for dependency identifiers.
Line 47 subtracts string values. This breaks TypeScript strict mode and yields NaN comparisons at runtime, so registry dependencies stay in insertion order instead of sorted order. The collectDeps test expects ["@termuijs/core", "@termuijs/motion", "@termuijs/widgets"].
Proposed fix
- return [...deps].sort((a, b) => a - b);
+ return [...deps].sort();📝 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.
| return [...deps].sort((a, b) => a - b); | |
| return [...deps].sort(); |
🤖 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 `@scripts/build-registry.ts` at line 47, Update the dependency sorting in
collectDeps to use a string comparator for identifiers rather than numeric
subtraction, ensuring alphabetical ordering such as `@termuijs/core`,
`@termuijs/motion`, and `@termuijs/widgets` while preserving deduplication.
1 participant
Description
This PR fixes real bugs found in the codebase:
.sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.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: #3516
Summary by CodeRabbit