-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix to _jsonContains and _jsonContainsTypes to handle callbacks in nested objects and arrays #180
Conversation
In _jsonContains and _jsonContainsTypes the arguments.callee.caller.name check returns false when the functions recurse when evaluating callbacks within nested objects. This fix will evaluate callbacks in nested objects when the callee.name is the same as the caller.name. This fix also evaluates callbacks within arrays by checking for the "array" type.
Are there still plans to merge these fixes? |
@@ -1217,7 +1217,7 @@ function _jsonContains(jsonBody, jsonTest) { | |||
var keyType = jsonTest[key].prototype; | |||
|
|||
// User-supplied callback (anonymous function) | |||
if(_toType(keyType) === "object" && arguments.callee.caller.name === "") { | |||
if(_toType(keyType) === "object" && (arguments.callee.caller.name === "" || arguments.callee.caller.name === arguments.callee.name)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the argument.callee...
part is removed in master; I'm not sure but it looks that it should be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed; the original problem I was fixing here was that the comparison to empty string broke recursion, which resulted in deep objects not being compared. I verified that the changes on master pass the (non-array) tests in this PR, so this change should now be unnecessary.
Looks that the very same issues was addressed in that commit: which is already on master. @jkwohlfahrt, could you please check if it helps? |
I'll close out this PR since it is pretty old, and master now has support for deep object comparisons. This PR also attempts to add support for array matchers, and in hindsight it is probably not appropriate to add this in the context of a bug fix. I'd suggest a new issue for array matchers could be opened up if that is something people would like to see, so we can discuss how (and if!) it might be implemented. |
Yes, matching whole array to with same element makes more sense. I've been confused by json-schema, because apparently there by default [{}, {}] checks element one by one. Thanks for providing the PR! |
This should fix #93 and #94