Open
Conversation
…and remove default property value for parameter bigNumberSupport in function extendedTypeOf() to ensure no unexpected parameter value will be processed
…is active, otherwise JSON.stringify() will still be used to ensure the tool will work as before when keeping bigNumberSupport deactivated
…ompared appropriately
…objects and ordinary numbers
Author
|
Hi, I just wanted to let you know that I now published my fork at npm (https://www.npmjs.com/package/big-json-diff). I hope this is okay for you. I still will appreciate if you merge my pull request. |
Collaborator
|
Hi @SebastianG77 ! Thank you, this seems like a useful change. However, we've converted our codebase over to ES6 and also cleaned up the way options are handled in index.js. If you can implement your change against the current codebase we can merge it in. |
ewoudenberg
requested changes
Apr 20, 2022
Collaborator
ewoudenberg
left a comment
There was a problem hiding this comment.
Hi @SebastianG77 ! Thank you, this seems like a useful change. However, we've converted our codebase over to ES6 and also cleaned up the way options are handled in index.js. If you can implement your change against the current codebase we can merge it in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Hi,
I need to diff JSON files which contain large numbers. Even though your module is generally very useful for comparing JSON files, it is unable to create proper diffs for JSON files containting large numbers, as it parses them by the ordinary JSON.parse() function. As a result, when comparing two files having the following JSON objects, the diff will not be recognized by this module:
file1:
{
"value": 3e+6000
}
file2:
{
"value": 3e+5000
}
However, I added optional BigNumber support. So if you now compare these two files and add the parameter -b, you will get the following diff:
{
- value: 3e+6000
+ value: 3e+5000
}
I know this is a very special use case as JSON objects usually do not have big numbers, but this feature is still useful as it extends the use of this module. It also should not break anything as my changes will only be activated if the options object contains the property bigNumberSupport. Moreover, all previous test cases are still running. I also added some additional test cases to check if this feature works and I am willing to add some more if desired.
Please let me know soon, if you are intersted in merging this pull request, as I also personally need this feature.
Otherwise I at least would like to publish the fork to npm by myself if you don't mind.
Looking forward for some response.