Skip to content
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

byProps/byState should match all keys #61

Closed
ooorayray opened this issue Oct 3, 2020 · 4 comments · Fixed by #62
Closed

byProps/byState should match all keys #61

ooorayray opened this issue Oct 3, 2020 · 4 comments · Fixed by #62
Labels
bug Something isn't working

Comments

@ooorayray
Copy link

Current behavior

When passing a matcher with multiple keys, .byProps/.byState returns nodes that match any one of the given keys.

Example:

const filtered = myComponent.byProps({ prop1: 123, prop2: 'abc' })
console.log(filtered)
/*
{
    name: 'MyComponent',
    props: {
        prop1: 123,
        prop2: 'def',
    },
    // ...
}
*/

Expected behavior

.byProps/.byState returns nodes that match ALL of the given keys.

Example:

const filtered = myComponent.byProps({ prop1: 123, prop2: 'abc' })
console.log(filtered)
/*
{
    name: 'MyComponent',
    props: {
        prop1: 123,
        prop2: 'abc',
    },
    // ...
}
*/

Failing test case: ooorayray@b502b83

@baruchvlz baruchvlz added bug Something isn't working good first issue Good for newcomers labels Oct 3, 2020
@baruchvlz
Copy link
Owner

Thanks for reporting and for the test case. I was able to pin point the error, I'm on my birthday holidays at the moment, but will fix this ASAP early next week along with #60 .

@baruchvlz
Copy link
Owner

Should be fixed in v1.9.0.

@rousku
Copy link

rousku commented Feb 10, 2022

@baruchvlz I still find current behavior unexpected. By writing const filtered = myComponent.byProps({ prop1: 123, prop2: 'abc' }) I expect all filtered nodes have props prop1: 123 AND prop2: 'abc'. I think this test case should fail because b doesn't have prop foo.

@baruchvlz
Copy link
Owner

@rousku The current match returns true if at least one of the properties match. There could be an optional flag to make it a strict match.

If you like you could open a PR and I would gladly review it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants