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

Expose api #13

Closed
wants to merge 6 commits into from
Closed

Expose api #13

wants to merge 6 commits into from

Conversation

phaelar
Copy link

@phaelar phaelar commented Oct 1, 2015

For shallow rendering sometimes we just want to find the components with the exact matching props without regard to its children. Consider the following example:

<Dropzone onDrop={this.onDrop} supportClick={true} className="bgp-attachment-dropzone" inputId={this.elementId('input')} multiple={true}>
              <div className="bgp-dropzone-instruction">
                Drag and drop files here
              </div>
</Dropzone>

In this case, we are not concerned whether its child is

<div className="bgp-dropzone-instruction">
                Drag and drop files here
              </div>

I have implemented findComponentsWithoutChildren and findComponentsWithoutChildren to facilitate such testing.

@glenjamin
Copy link
Owner

The React 0.14 changes are not relevant to this PR, so please leave those out.

Also, please leave the == changes out, they're not relevant to this either - the .eslintrc file has the rules which this requires.

As for the actual change - is there a reason you can't just use findComponentLike ?

We've also got some discussion in #9 about what direction the API is going to take - feel free to weight on in that.

@clouddra
Copy link

clouddra commented Oct 2, 2015

For example:

<Parent name='hello' item='some_item' >
  <div>something</div>
  <div>something else</div>
</Parent>

In the above scenario, findComponentLike('Parent', {name: 'hello'}) will still return the above component as it is a subset. What we really want is to assert that the matched object does not contain extra props that were accidentally added in the code (i.e. props.item).

The plural alternative findComponentsLike('Parent', {name: 'hello'}) is also a convenience method for us to assert that there are multiple matching components

<Parent name='hello' item='some_item' >
  <div>something</div>
</Parent>
<Parent name='hello' item='some_item' >
  <div>something</div>
</Parent>

For the above example, we could simply assert findComponentsLike('Parent', {name: 'hello'}).length == 2. In fact, we use findComponentsLike for all our tests, even for only single components, (i.e. findComponentsLike('Parent', {name: 'hello'}).length == 1 to really test there is only 1 of that component.

@glenjamin
Copy link
Owner

Ok, I can see that is a valid scenario - although I'd recommend against exact matches in general as they lead to brittle tests.

How about a similar feature to http://sinonjs.org/docs/#sinon-match-api ?

findComponent("Name", { prop1: 123, children: SkinDeep.any)

This is more general, but should achieve the same thing.

You'll also be able to use the predicate API discussed in #9 to use any matching approach at all, including sinon.match

Your point about .length === 1 is useful - I'll take that on board when expanding the chai plugin.

@clouddra
Copy link

clouddra commented Oct 2, 2015

SkinDeep.any is a good suggestion. Just wondering about how can we go about implementing .any. sinon.match.any is just a simple wrapper which returns true but I am not sure if it is a workable solution for this case.

@glenjamin
Copy link
Owner

The default implementation of Like needs to consider this value as "always match".

I can fold this into the changes coming in #9 if you don't mind waiting a little bit - but would take a PR which added SkinDeep.any support to findComponents?Like if you want it sooner

@clouddra
Copy link

clouddra commented Oct 3, 2015

I will wait for #9 then. Thanks for the help!

@glenjamin
Copy link
Owner

Closing this, but leaving #31 open for tracking

@glenjamin glenjamin closed this Nov 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants