Skip to content

fix: prevent silent precision loss for integers beyond MAX_SAFE_INTEGER#155

Open
terminalchai wants to merge 1 commit into
unjs:mainfrom
terminalchai:fix/precision-loss-large-integers
Open

fix: prevent silent precision loss for integers beyond MAX_SAFE_INTEGER#155
terminalchai wants to merge 1 commit into
unjs:mainfrom
terminalchai:fix/precision-loss-large-integers

Conversation

@terminalchai
Copy link
Copy Markdown

@terminalchai terminalchai commented Mar 14, 2026

Summary

Closes #152

destr uses JSON.parse to convert numeric-looking strings to numbers. When a string represents an integer larger than Number.MAX_SAFE_INTEGER, JSON.parse silently loses precision:

destr('9007199254740993') // → 9007199254740992  ❌ (silent data corruption)

This affects large IDs, counters, and any integer-valued string coming through Nitro/Nuxt runtimeConfig, h3 query params, etc.

Fix

After JSON.parse succeeds, if the result is a non-safe integer (!Number.isSafeInteger(result)), return the original string instead of the corrupted number:

// src/index.ts
const result = JSON.parse(value);
if (
  typeof result === "number" &&
  Number.isInteger(result) &&
  !Number.isSafeInteger(result)
) {
  if (options.strict) {
    throw new SyntaxError(`[destr] Loss of precision for large integer string: "${value}"`);
  }
  return value as T; // keep the string — precision cannot be guaranteed
}
return result;
  • Safe integers (<= Number.MAX_SAFE_INTEGER) continue to parse as numbers — no behaviour change.
  • safeDestr throws a descriptive SyntaxError for large integers rather than silently corrupting.

Tests

Two new test cases added to test/index.test.ts:

  • destr returns the original string for 9007199254740992, 9007199254740993, -9007199254740993
  • safeDestr throws "[destr] Loss of precision for large integer string" in strict mode

All 24 tests pass.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of large integer values during parsing to prevent silent precision loss
    • In strict mode, invalid integer values now raise an error; otherwise returns the original string value

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fa1e9906-d4d7-49a4-8da0-b8f583faa866

📥 Commits

Reviewing files that changed from the base of the PR and between 469ec7f and 3f868e5.

📒 Files selected for processing (2)
  • src/index.ts
  • test/index.test.ts

📝 Walkthrough

Walkthrough

The update adds a safety guard to prevent silent precision loss when parsing numeric strings that exceed JavaScript's maximum safe integer limit (Number.MAX_SAFE_INTEGER). In strict mode, the function throws an error; otherwise, it returns the original string value unchanged.

Changes

Cohort / File(s) Summary
Large Integer Safety Guard
src/index.ts
Added conditional logic to detect parsed values that are numeric integers beyond safe range. Returns original string or throws SyntaxError in strict mode to prevent precision loss from unsafe number coercion.
Test Coverage
test/index.test.ts
Added two test cases: one verifying large integer strings are returned unchanged in normal mode, another confirming strict mode throws a SyntaxError for the same scenario.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hop, a skip, precision's back!
No more integers lost in the stack,
Safe and sound, our numbers stay true,
Nine-zero-zero-seven preserved for you!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main fix: preventing silent precision loss for large integers beyond MAX_SAFE_INTEGER.
Linked Issues check ✅ Passed The code changes fully address issue #152: large integer strings are preserved as strings in normal mode and throw a descriptive error in strict mode.
Out of Scope Changes check ✅ Passed All changes are directly related to preventing precision loss for large integers; no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

destr incorrectly parses large numeric strings into unsafe Numbers, causing precision loss

1 participant