fix(data): avoid crashing with BigInt values #19847
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Brief Information
This pull request is in the type of:
What does this PR do?
isFinite
withNumber.isFinite
and unary (+
) withNumber()
conversion so that a simple chart doesn't blow up when passing in aBigInt
FixedRelated issuesDetails
Before: What was the problem?
When passing a
BigInt
data column to a chart, the chart blows up.👇
This is happens because of the way
isFinite
,isNaN
, and+
(unary operator) coerces the values.isFinite
andisNaN
implicitly converts the values in a way that's incompatible withBigInt
.The
Number.isFinite()
andNumber.isNaN()
methods work in a more predictable way by not casting non-numeric values implicitly - We can do that explicitly usingNumber()
which can be more more performant) and does work withBigInt
The unary
+
operator conversion will likely never be supported forBigInt
for compatibility reasons with ASM, whileNumber()
does (with possible loss of precision)After: How does it behave after the fixing?
Chart is rendered without crashing 🚀
Document Info
One of the following should be checked.
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information
Please let me know if you'd like me to address some of the conversion issues in other places of the codebase too. That would require quite a few changes though; I've kept this PR small to increase its chances of being merged 👀
Thank you 🙏