fix: parsing decimals and scientific notation #94
Conversation
Codecov Report
@@ Coverage Diff @@
## main #94 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 96 96
Branches 39 38 -1
=========================================
Hits 96 96
|
| @@ -5,7 +5,7 @@ const suspectProtoRx = | |||
| const suspectConstructorRx = | |||
| /"(?:c|\\u0063)(?:o|\\u006[Ff])(?:n|\\u006[Ee])(?:s|\\u0073)(?:t|\\u0074)(?:r|\\u0072)(?:u|\\u0075)(?:c|\\u0063)(?:t|\\u0074)(?:o|\\u006[Ff])(?:r|\\u0072)"\s*:/; | |||
|
|
|||
| const JsonSigRx = /^\s*["[{]|^\s*-?\d[\d.]{0,14}\s*$/; | |||
| const JsonSigRx = /^\s*["[{]|^\s*-?\d{1,16}(\.\d{1,17})?([Ee][+-]?\d+)?\s*$/; | |||
There was a problem hiding this comment.
There is only a small issue with this. It seems maximum 16 digits cannot be combined with 17 decimal places at the same time. So 1.1234567812345678 is valid (well, kinda it will rounded to 1.1234567812345677) also 1234567812345678 is valid but 1234567812345678.1234567812345678 will be trimmed to 1234567812345678.
I don't think for this particular regex it would be an issue because JSON.parse will normalize it anyway...
There was a problem hiding this comment.
Yes, the regex here are slightly rough, and being able to cover all the right scenarios should be enough, leaving the rest to JSON.parse
There was a problem hiding this comment.
@pi0 Sorry to disturb, I ran into a new problem today, in nodejs numbers can have more than 17 decimals (Math.random() gave me this result 0.018178923413311843), so in order to cover more scenarios, can we remove the digit limit and just match with more than 1 digit, for example:
- const JsonSigRx = /^\s*["[{]|^\s*-?\d{1,16}(\.\d{1,17})?([Ee][+-]?\d+)?\s*$/;
+ const JsonSigRx = /^\s*["[{]|^\s*-?\d+(\.\d+)?([Ee][+-]?\d+)?\s*$/;
π Linked issue
Closes #93
β Type of change
π Description
Fix parsing of multi-digit numbers and scientific notation.
If anything needs tweaking don't hesitate to point it out, you can always edit it too :)
π Checklist