Fix: bigint and number conflicts #2808
Conversation
Console (appwrite/console)Project ID: Tip Sites auto-generate unique domains with the pattern https://randomstring.appwrite.network |
WalkthroughAdds bigint and large-number support across helpers and UI: new constants and functions (LARGE_NUMBER_THRESHOLD, LARGE_NUMBER_THRESHOLD_NUM, coerceToNumber, toExponential), widened numeric props in inputNumber to accept number | bigint, and column display/tooltip formatting that renders large values in exponential form. generateFields signature was expanded (databaseId, tableId, databaseType) and integer bound handling now coerces values before safe-range checks. DocumentsDB was removed from the project-scoped SDK surface and related route handlers. Several type/assertion and prop-forwarding adjustments and dependency updates were applied; tests for toExponential were added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🤖 Fix all issues with AI agents
In `@src/lib/elements/forms/inputNumber.svelte`:
- Line 8: The exported props value, min, and max in inputNumber.svelte currently
allow number | bigint but are passed to Input.Number which expects number;
import coerceToNumber from $lib/helpers/numbers and use it to convert each prop
before binding to <Input.Number> (e.g., replace direct bindings of value, min,
max with coerceToNumber(value), coerceToNumber(min), coerceToNumber(max));
ensure the exported props keep their types but the component receives plain
numbers.
🧹 Nitpick comments (1)
src/lib/helpers/numbers.ts (1)
45-47: Consider documenting or handling potential precision loss for large bigint values.
Number(val)silently loses precision whenvalexceedsNumber.MAX_SAFE_INTEGER. SinceisWithinSafeRangealready exists in this file, consider either:
- Adding a brief JSDoc comment noting this limitation, or
- Combining with
isWithinSafeRangeto warn/handle unsafe coercions📝 Optional: Add documentation
+/** + * Coerces a number or bigint to a JavaScript number. + * Note: Values outside Number.MAX_SAFE_INTEGER may lose precision. + */ export function coerceToNumber(val: number | bigint) { return Number(val); }
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/lib/components/sidebar.svelte`:
- Line 104: The component removed rest-attribute forwarding which breaks
consumers expecting attributes/events; restore Svelte rest props by forwarding
$$restProps to the child element (Sidebar.Base) so attributes like id, data-*,
aria-*, or custom event handlers are preserved—update the Sidebar.Base usage
(the element with bind:state, resizable, on:resize and updateSidebarState) to
include the spread of $$restProps rather than using or omitting $$props.
In `@src/lib/helpers/numbers.ts`:
- Around line 54-79: The toExponential function emits a trailing decimal point
for non-zero BigInt when fractionDigits is 0 (e.g., "9.e+14"); add a fast path
inside toExponential (before building fraction) that checks for typeof value ===
'bigint' (or after the existing value === 0n check) and if fractionDigits === 0
return `${sign}${digits[0]}e+${digits.length - 1}` (i.e., no decimal separator),
otherwise continue with the existing fraction construction; reference the
toExponential function, the variables bigIntString, sign, digits, and
fractionDigits when applying the change.
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte:
- Around line 170-178: The code crashes when calling BigInt(Math.abs(num)) on
non-integer numbers; update formatLargeNumber and the other occurrences that
currently call BigInt(Math.abs(...)) (the places computing abs/threshold for
min/max display) to be type-safe: if typeof value === 'bigint' compute abs via
value < 0n ? -value : value and compare to LARGE_NUMBER_THRESHOLD using bigint;
if typeof value === 'number' use Math.abs(value) and compare to
LARGE_NUMBER_THRESHOLD as a number (no BigInt conversion) and return
num.toString() or toExponential(num) accordingly. Ensure all comparisons and
returns preserve the original type handling for number vs bigint and replace any
BigInt(Math.abs(...)) usages with this branching logic in the functions/methods
where min/max/formatting is done (e.g., formatLargeNumber and the min/max
formatting helpers).
🧹 Nitpick comments (1)
src/lib/helpers/numbers.test.ts (1)
88-122: Consider coveringfractionDigits = 0for non‑zero values.
This would lock in the exact formatting that matchesNumber#toExponential(0)and the BigInt branch.✅ Suggested test addition
test('handle zero correctly', () => { expect(toExponential(0)).toEqual('0.00e+0'); expect(toExponential(0n)).toEqual('0.00e+0'); expect(toExponential(0n, 0)).toEqual('0e+0'); }); +test('format non-zero values with zero fraction digits', () => { + expect(toExponential(12345, 0)).toEqual('1e+4'); + expect(toExponential(12345n, 0)).toEqual('1e+4'); +}); + test('validate fractionDigits range', () => { expect(() => toExponential(123, -1)).toThrow(RangeError); expect(() => toExponential(123, 101)).toThrow(RangeError); expect(() => toExponential(123n, -1)).toThrow(RangeError); });
.../project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/routes/`(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte:
- Around line 201-202: The comparisons of min and max against
Number.MIN_SAFE_INTEGER / Number.MAX_SAFE_INTEGER can throw when min/max are
bigint; update the hasValidMin and hasValidMax checks to first guard the type of
min/max (use typeof min === 'bigint' and typeof max === 'bigint') and perform
bigint-safe comparisons (or convert the Number constants to BigInt) similar to
the existing bigint guards used later in this block (refer to the min, max,
hasValidMin, hasValidMax variables and the bigint handling around lines where
typeof checks are performed) so that mixing BigInt and Number is avoided.
🧹 Nitpick comments (2)
src/lib/helpers/numbers.ts (1)
52-67: Docstring doesn't reflect full function capability.The docstring says "Converts a bigint to exponential notation" but the function handles both
numberandbigint. Consider updating for accuracy.Suggested fix
-/** - * Converts a bigint to exponential notation without precision loss. - */ +/** + * Converts a number or bigint to exponential notation. + * For bigint, avoids precision loss by using string manipulation. + */ export function toExponential(value: number | bigint, fractionDigits: number = 2): string {src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/+page.svelte (1)
209-237: Consider extracting repeated threshold check into a helper.The
shouldShowTooltiplogic (abs >= threshold with type guards) is repeated three times. A small helper would reduce duplication:Example helper
function exceedsThreshold(val: number | bigint): boolean { return typeof val === 'bigint' ? (val < 0n ? -val : val) >= LARGE_NUMBER_THRESHOLD : Math.abs(val) >= LARGE_NUMBER_THRESHOLD_NUM; }This is optional since the current approach is readable and self-contained.
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.