fix: resolve 2 bugs - #3376
Conversation
github-actions
Bot
added
type:bug
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes make numeric parsing explicit across examples, bound numeric HTML entity payloads, and replace default sorting with numeric comparators in UI and build logic. ChangesNumeric parsing fixes
Numeric sorting fixes
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 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-35: Update the numeric entity decoding branch in the RSS text
rendering function to require a non-empty entity, parse the complete
entity.slice(...) value, and validate codePoint is within 0 through 0x10FFFF
inclusive before calling String.fromCodePoint. Apply the same range validation
to hexadecimal and decimal entities, returning match for invalid values to
prevent RangeError.
In `@packages/ui/src/TreeSelect.ts`:
- Around line 185-186: Replace the numeric subtraction comparators in
_valuesEqual at packages/ui/src/TreeSelect.ts lines 185-186 with lexical string
ordering for both sorted arrays. Apply the same string-order comparator in
collectDeps at scripts/build-registry.ts line 47 so package specifiers use a
canonical order; no other changes are needed.
🪄 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: 8eefa779-317f-4598-b503-65c470f98ca6
📒 Files selected for processing (5)
examples/rss-reader/src/index.tsxexamples/showcase/src/index.tsxexamples/widget-gallery/src/index.tspackages/ui/src/TreeSelect.tsscripts/build-registry.ts
| const codePoint = Number.parseInt(entity.slice(2, 10), 16); | ||
| return Number.isFinite(codePoint) ? String.fromCodePoint(codePoint) : match; | ||
| } | ||
|
|
||
| if (entity.startsWith('#')) { | ||
| const codePoint = Number.parseInt(entity.slice(1), 10); | ||
| const codePoint = Number.parseInt(entity.slice(1, 10), 10); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching index.tsx:"
fd -a '^index\.tsx$' . | sed 's#^\./##' || true
file="$(fd '^index\.tsx$' examples | head -n 1 || true)"
if [ -n "${file:-}" ]; then
echo "Target file: $file"
wc -l "$file"
sed -n '1,120p' "$file" | nl -ba
fi
echo
echo "Search for numeric entity decode helpers:"
rg -n "fromCodePoint|parseInt\\(entity|entity\\.startsWith|#\\&|`#x`|#\\d" . --glob '*.{ts,tsx}' || trueRepository: Karanjot786/TermUI
Length of output: 2216
🌐 Web query:
MDN String.fromCodePoint valid Unicode code point RangeError code points range
💡 Result:
The String.fromCodePoint static method in JavaScript expects one or more arguments, each representing a valid Unicode code point [1][2]. A valid Unicode code point is an integer within the inclusive range of 0 to 0x10FFFF (decimal 1,114,111) [1][3]. A RangeError is thrown if any of the provided arguments fail to meet these criteria [1][3]. Specifically, the method throws a RangeError in the following scenarios: - The value is not an integer (e.g., 3.14 or other non-integral numbers) [1][3][4]. - The value is less than 0 [1][3][4]. - The value is greater than 0x10FFFF [1][3][4]. - The value is NaN [3][5]. As defined in the ECMAScript specification, these checks are performed after the input is converted to a number [4][6]. If an argument is out of range or not an integral number, the operation fails and the RangeError is thrown [4][6].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/fromCodePoint
- 2: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/global_objects/string/fromcodepoint/index.md
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Not_a_valid_code_point
- 4: https://tc39.es/ecma262/2023/multipage/text-processing.html
- 5: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/errors/not_a_valid_code_point/index.md
- 6: https://tc39.es/ecma262/pr/3783/multipage/text-processing.html
🌐 Web query:
MDN Number.parseInt radix overflow to Number.MAX_SAFE_INTEGER hex parsing
💡 Result:
The parseInt() (and Number.parseInt()) function in JavaScript does not trigger an "overflow" or error when parsing a string that exceeds Number.MAX_SAFE_INTEGER [1][2]. Instead, it parses the string and returns the closest representable Number value [1]. Because JavaScript numbers use IEEE 754 double-precision floating-point format, they can only represent integers exactly within the range of -(2^53 - 1) to 2^53 - 1 (Number.MAX_SAFE_INTEGER) [3]. If you parse a hexadecimal string that represents a value larger than this, parseInt() will return a rounded approximation of that value [1][2]. Key points regarding this behavior: 1. No Exception: parseInt() will not throw an error or return NaN simply because a number is larger than Number.MAX_SAFE_INTEGER [1][2]. 2. Precision Loss: Once the value exceeds the safe integer range, precision is lost. Subsequent integers may be rounded to the same representable floating-point number, making them indistinguishable during equality checks [2][3]. 3. Hexadecimal Parsing: When provided with a hex string (e.g., starting with 0x), parseInt(string, 16) correctly interprets the digits but remains subject to the same floating-point limitations as any other number parsing [4][5]. 4. Recommended Alternative: If you need to work with integers larger than Number.MAX_SAFE_INTEGER without losing precision, use the BigInt type [3][5]. You can parse large hexadecimal strings using BigInt("0x" + hexString) or BigInt(string) if the radix is handled appropriately [5]. For example: // This will result in precision loss const largeHex = "0x20000000000001"; // Greater than Number.MAX_SAFE_INTEGER const parsed = parseInt(largeHex, 16); console.log(parsed > Number.MAX_SAFE_INTEGER); // true console.log(parsed); // Returns the rounded Number representation // For accurate parsing, use BigInt const bigIntVal = BigInt(largeHex); console.log(bigIntVal); // Returns the exact BigInt value
Citations:
- 1: https://stackoverflow.com/questions/35143869/how-is-the-parseint-in-javascript-defined-to-handle-large-numbers-is-there-a
- 2: https://stackoverflow.com/questions/58406880/calling-parseint-with-a-string-which-represents-a-value-larger-than-number-max-s
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
- 4: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt
- 5: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global%5FObjects/parseInt
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="examples/rss-reader/src/index.tsx"
echo "Target file: $file"
wc -l "$file"
sed -n '1,110p' "$file" | awk '{printf "%3d:\t%s\n", NR, $0}'
echo
echo "Search for replace entity function usages:"
rg -n "replace|entity|decode|fromCodePoint|parseInt" "$file" || true
echo
echo "Behavioral probe for current decoding behavior:"
node - <<'JS'
for (const input of ['&`#x110000`;', '&`#999999999`;', '&`#x00000041dead`;', '&`#x0041`;', '&`#X0041`;', '&`#0041`;']) {
let match = String(input);
try {
match = match.replace(
/&#(\d{1,9});|&`#x`(\d{1,9});|&`#X`(\d{1,9});/gi,
(_, decimal, lowercaseHex, uppercaseHex) => {
const entity = (decimal || lowercaseHex || uppercaseHex);
const codePoint = Number.parseInt(entity.slice(Math.max(0, entity.length - Math.min(Math.max(0, entity.length), 9)), Math.max(0, entity.length - Math.min(Math.max(0, entity.length), 9)) || entity), 16);
return Number.isFinite(codePoint) ? String.fromCodePoint(parseInt(isNaN(decimal) ? lowercaseHex || uppercaseHex : decimal, 16)) : match;
}
);
} catch (err) {
console.log(`${input} => throws ${err.name}`);
console.log(err.message);
continue;
}
console.log(`${input} => ${match}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 5435
Reject invalid numeric entities before decoding.
Number.isArrayFinite(codePoint) does not validate the Unicode range for String.fromCodePoint, so values above 0x10FFFF throw RangeError during RSS text rendering. Accept only non-empty matches, parse the full entity.slice(...) value, and keep the value in 0 through 0x10FFFF inclusive before decoding.
🤖 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 - 35, Update the numeric
entity decoding branch in the RSS text rendering function to require a non-empty
entity, parse the complete entity.slice(...) value, and validate codePoint is
within 0 through 0x10FFFF inclusive before calling String.fromCodePoint. Apply
the same range validation to hexadecimal and decimal entities, returning match
for invalid values to prevent RangeError.
| const sortedA = [...a].sort((a, b) => a - b); | ||
| const sortedB = [...b].sort((a, b) => a - b); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files of interest:\n'
git ls-files | rg '(^packages/ui/src/TreeSelect\.ts$|^scripts/build-registry\.ts$|^scripts/build-registry\.test\.ts$)' || true
printf '\n--- TreeSelect relevant excerpt ---\n'
nl -ba packages/ui/src/TreeSelect.ts | sed -n '160,205p'
printf '\n--- build-registry relevant excerpt ---\n'
nl -ba scripts/build-registry.ts | sed -n '1,80p'
printf '\n--- tests relevant excerpt if present ---\n'
if [ -f scripts/build-registry.test.ts ]; then
nl -ba scripts/build-registry.test.ts | sed -n '100,155p'
fi
printf '\n--- type usage around sorted values ---\n'
rg -n "sort\\(|valuesEqual|collectDeps|deps|deps" packages/ui/src/TreeSelect.ts scripts/build-registry.ts scripts/build-registry.test.ts || true
printf '\n--- Behavioral probe: runtime comparator results for strings ---\n'
node - <<'JS'
const tests = [
[['apple', 'banana', 'cherry'], 'strings'],
[['001', '1', '01'], 'leading-zeros strings'],
[['0', '2', '1'], 'digit strings'],
[['0.1', '0.2', '1'], 'decimal strings'],
[['', 'apple'], 'string/empty'],
[['a', 1], 'mixed string/number'],
];
for (const [arr, label] of tests) {
const a = [...arr].sort((a, b) => a - b);
const b = [...arr].sort((a, b) => String(a).localeCompare(String(b)));
console.log(`${label}: ${JSON.stringify([...arr])} -> numeric=${JSON.stringify(a)} lexical=${JSON.stringify(b)}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- TreeSelect relevant excerpt ---\n'
awk '{printf "%6d\t%s\n", NR, $0}' packages/ui/src/TreeSelect.ts | sed -n '160,205p'
printf '\n--- build-registry relevant excerpt ---\n'
awk '{printf "%6d\t%s\n", NR, $0}' scripts/build-registry.ts | sed -n '1,80p'
printf '\n--- tests relevant excerpt if present ---\n'
awk '{printf "%6d\t%s\n", NR, $0}' scripts/build-registry.test.ts | sed -n '100,155p'
printf '\n--- Search relevant usage ---\n'
rg -n "sort\\(|valuesEqual|collectDeps|deps|deps" packages/ui/src/TreeSelect.ts scripts/build-registry.ts scripts/build-registry.test.ts || true
printf '\n--- Behavioral probe: runtime comparator results for strings ---\n'
node - <<'JS'
const tests = [
[['apple', 'banana', 'cherry'], 'strings'],
[['001', '1', '01'], 'leading-zeros strings'],
[['0', '2', '1'], 'digit strings'],
[['0.1', '0.2', '1'], 'decimal strings'],
[['', 'apple'], 'string/empty'],
[['a', 1], 'mixed string/number'],
];
for (const [arr, label] of tests) {
const a = [...arr].sort((a, b) => a - b);
const b = [...arr].sort((a, b) => String(a).localeCompare(String(b)));
console.log(`${label}: ${JSON.stringify([...arr])} -> numeric=${JSON.stringify(a)} lexical=${JSON.stringify(b)}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- TreeSelect relevant excerpt ---'
awk '{printf "%6d\t%s\n", NR, $0}' packages/ui/src/TreeSelect.ts | sed -n '160,205p'
echo
echo '--- build-registry relevant excerpt ---'
awk '{printf "%6d\t%s\n", NR, $0}' scripts/build-registry.ts | sed -n '1,80p'
echo
echo '--- tests relevant excerpt if present ---'
awk '{printf "%6d\t%s\n", NR, $0}' scripts/build-registry.test.ts | sed -n '100,155p'
echo
echo '--- Search relevant usage ---'
rg -n "sort\(|valuesEqual|collectDeps|deps|deps" packages/ui/src/TreeSelect.ts scripts/build-registry.ts scripts/build-registry.test.ts || true
echo
echo '--- Behavioral probe: runtime comparator results for strings ---'
node - <<'JS'
const tests = [
[['apple', 'banana', 'cherry'], 'strings'],
[['001', '1', '01'], 'leading-zeros strings'],
[['0', '2', '1'], 'digit strings'],
[['0.1', '0.2', '1'], 'decimal strings'],
[['', 'apple'], 'string/empty'],
[['a', 1], 'mixed string/number'],
];
for (const [arr, label] of tests) {
const a = [...arr].sort((a, b) => a - b);
const b = [...arr].sort((a, b) => String(a).localeCompare(String(b)));
console.log(`${label}: ${JSON.stringify([...arr])} -> numeric=${JSON.stringify(a)} lexical=${JSON.stringify(b)}`);
}
JSRepository: Karanjot786/TermUI
Length of output: 9926
Use string comparison for these comparators.
The comparators are called on string[]; numeric subtraction is out of the TypeScript strict contract and compares nonnumeric strings by NaN.
packages/ui/src/TreeSelect.ts#L185-L186: use string ordering so_valuesEqualstays order-independent for values like["001", "1", "01"].scripts/build-registry.ts#L47: use string ordering for package specifiers socollectDepskeeps a canonical result.
📍 Affects 2 files
packages/ui/src/TreeSelect.ts#L185-L186(this comment)scripts/build-registry.ts#L47-L47
🤖 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, Replace the numeric
subtraction comparators in _valuesEqual at packages/ui/src/TreeSelect.ts lines
185-186 with lexical string ordering for both sorted arrays. Apply the same
string-order comparator in collectDeps at scripts/build-registry.ts line 47 so
package specifiers use a canonical order; no other changes are needed.
Source: Coding guidelines
saurabhhhcodes
changed the title
1 participant
Description
This PR fixes real bugs found in the codebase:
.map()on an undefined collection threwTypeError; now falls back to[]..map()on an undefined collection threwTypeError; now falls back to[].Type of Change
How Has This Been Tested?
Checklist
Related Issue
Ref: #3383