fix: resolve 4 bugs in termui - #3565
Conversation
📝 WalkthroughWalkthroughThe changes make tab-key parsing explicitly base 10, add ChangesNumeric handling fixes
Estimated code review effort: 2 (Simple) | ~5 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)
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: 1
🤖 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/ui/src/TreeSelect.ts`:
- Around line 185-186: Update the sortedA and sortedB sort calls to use a typed
numeric comparator that converts each string element to a number before
subtraction. Ensure both arrays use numeric ordering so equivalent values such
as “2” and “10” compare consistently under strict TypeScript checking.
🪄 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: f6523553-2b9f-459c-a5f7-fd903e330638
📒 Files selected for processing (4)
examples/showcase/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/Switch.tspackages/ui/src/TreeSelect.ts
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].sort(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'function _valuesEqual|sortedA|sortedB' packages/ui/src/TreeSelect.ts
while IFS= read -r file; do
echo "== $file =="
rg -n -C 2 '"strict"|"noImplicitAny"|"extends"|"include"' "$file" || true
done < <(fd -a 'tsconfig.*\.json$' .)Repository: Karanjot786/TermUI
Length of output: 600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== TreeSelect relevant section =="
sed -n '170,192p' packages/ui/src/TreeSelect.ts
echo
echo "== TypeScript config files =="
git ls-files '*tsconfig*.json' | while IFS= read -r file; do
echo "-- $file --"
sed -n '1,220p' "$file"
done
echo
echo "== tsconfig strict flags references =="
rg -n '"strict"|"extends"|"include"|"compilerOptions"' $(git ls-files '*tsconfig*.json') || true
echo
echo "== Read-only behavior probe for _valuesEqual with ['2','10'] and ['10','2'] =="
node - <<'JS'
function valuesEqual(a, b) {
const sortedA = [...a].sort((a, b) => a - b);
const sortedB = [...b].sort();
if (a.length !== b.length) return false;
for (let i = 0; i < sortedA.length; i++) {
if (sortedA[i] !== sortedB[i]) return false;
}
return true;
}
console.log(JSON.stringify({
inputA: ['2', '10'],
inputB: ['10', '2'],
equal: valuesEqual(['2', '10'], ['10', '2'])
}));
JSRepository: Karanjot786/TermUI
Length of output: 26059
Use a typed numeric comparator for both array sorts.
packages/ui/tsconfig.json enables strict, so subtracting the string[] elements in sortedA.sort((a, b) => a - b) is a type error. sortedB.sort() also does default lexicographic sorting, so ['2', '10'] and ['10', '2'] are treated as unequal.
Proposed fix
- const sortedA = [...a].sort((a, b) => a - b);
- const sortedB = [...b].sort();
+ const compareNumeric = (left: string, right: string): number =>
+ Number(left) - Number(right);
+ const sortedA = [...a].sort(compareNumeric);
+ const sortedB = [...b].sort(compareNumeric);📝 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 sortedA = [...a].sort((a, b) => a - b); | |
| const sortedB = [...b].sort(); | |
| const compareNumeric = (left: string, right: string): number => | |
| Number(left) - Number(right); | |
| const sortedA = [...a].sort(compareNumeric); | |
| const sortedB = [...b].sort(compareNumeric); |
🧰 Tools
🪛 GitHub Actions: CI / 0_build-and-test.txt
[error] 185-185: TypeScript TS2362: The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type. The DTS build failed while running 'tsup'.
[error] 185-185: TypeScript TS2363: The right-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type. The DTS build failed while running 'tsup'.
🪛 GitHub Actions: CI / build-and-test
[error] 185-185: TypeScript DTS build failed: the left-hand side of an arithmetic operation must be any, number, bigint, or an enum type (TS2362), and the right-hand side must satisfy the same requirement (TS2363). Failed command: bun run build (tsup).
🤖 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/TreeSelect.ts` around lines 185 - 186, Update the sortedA and
sortedB sort calls to use a typed numeric comparator that converts each string
element to a number before subtraction. Ensure both arrays use numeric ordering
so equivalent values such as “2” and “10” compare consistently under strict
TypeScript checking.
Source: Coding guidelines
1 participant
Description
This PR fixes real bugs found in the codebase:
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..sort()coerces elements to strings, so[10, 9, 2]sorts as[10, 2, 9]; numeric comparator sorts correctly.parseInt: without10, strings like'0x1F'or'08'parse in unintended bases.Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3564
Summary by CodeRabbit