digitalbazaar / digitalbazaar/oid4-client
Consider using `Number.isSafeInteger(i)`
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 2
- Forks
- 2
- Avg merge
- 16m
- Merged PRs (30d)
- 1
Description
- Does this need more edge case checks?
- Might consider at least having a zero and negative int check. Probably Number.MIN/MAX_SAFE_INTEGER too.
- Does this need the reverse checks of data with a plain int and query with an int string? I think the code would handle that?
- Parsed JSON itself with native ints will (probably?) be limited to Number.MIN/MAX_SAFE_INTEGER. But if both sides had strings with non safe int values, I think this starts to lose precision and would cause matching failures. In any case, havoc will probably happen. Is that important to handle here?
Example:
> Number.MAX_SAFE_INTEGER
9007199254740991
> a = parseInt('9007199254740992')
9007199254740992
> b = parseInt('9007199254740993')
9007199254740992
> a.toString()
'9007199254740992'
> b.toString()
'9007199254740992'
> a.toString() === b.toString()
true
- One solution might be to use
BigInt(x), but that's introducing radix prefix issues and returning BigInts vs ints for comparison and no doubt has odd issues. - Another solution is to specifically not handle these issues and after the parseInt, check if Number.isSafeInteger(i) is false, then return x. Would need some tests to make sure that works and simply refuses to match those non-safe string values for now.
Originally posted by @davidlehn in https://github.com/digitalbazaar/oid4-client/pull/79#pullrequestreview-4386185039
We probably only want the Number.isSafeInteger(i) check, not any BigInt checks. We're only interested, I think, in converting integers for array indexing processing and interoperable JSON numbers.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by locating the parseInt conversion used for array-indexing processing and review how JSON numbers and integer strings are compared. Add edge-case tests around Number.isSafeInteger(i), including safe and non-safe values, and verify that non-safe integer strings do not produce incorrect matches.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- api
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100