Fix Mount CSS prop selector for parity with Shallow

[travisperson]

Selector such as Foo[bar="baz"] previously would not return any components when rendering with mount.

Resolves #534

[blainekasten]

lgtm

[travisperson]

I think this is going to introduce similar issues as #377. I went a head and passed the props all the way down to the child div and the ReactWrapper test now fails as it finds two items.

diff --git a/test/ReactWrapper-spec.jsx b/test/ReactWrapper-spec.jsx
index abb1aee..305f388 100644
--- a/test/ReactWrapper-spec.jsx
+++ b/test/ReactWrapper-spec.jsx
@@ -386,16 +386,16 @@ describeWithDOM('mount', () => {

     it('should find React Component based on a components prop via css selector', () => {
       class Foo extends React.Component {
-        render() { return <div />; }
+        render() { return <div {...this.props} />; }
       }
       const wrapper = mount(
         <div>
-          <Foo bar="baz" />
+          <Foo role="button" />
         </div>
       );

-      expect(wrapper.find('[bar]')).to.have.length(1);
-      expect(wrapper.find('Foo[bar="baz"]')).to.have.length(1);
+      expect(wrapper.find('[role]')).to.have.length(1);
+      expect(wrapper.find('Foo[role="button"]')).to.have.length(1);
     });

     it('should not find components with invalid attributes', () => {
diff --git a/test/ShallowWrapper-spec.jsx b/test/ShallowWrapper-spec.jsx
index 6ec1331..5f2f753 100644
--- a/test/ShallowWrapper-spec.jsx
+++ b/test/ShallowWrapper-spec.jsx
@@ -483,16 +483,16 @@ describe('shallow', () => {

     it('should find a React Component based on a components prop via css selector', () => {
       class Foo extends React.Component {
-        render() { return <div />; }
+        render() { return <div {...this.props} />; }
       }
       const wrapper = shallow(
         <div>
-          <Foo bar="baz" />
+          <Foo role="button" />
         </div>
       );

-      expect(wrapper.find('[bar]')).to.have.length(1);
-      expect(wrapper.find('Foo[bar="baz"]')).to.have.length(1);
+      expect(wrapper.find('[role]')).to.have.length(1);
+      expect(wrapper.find('Foo[role="button"]')).to.have.length(1);
     });

     it('should find components with multiple matching react props', () => {

  1) (uses jsdom) mount .find(selector) should find React Component based on a components prop via css selector:
     AssertionError: expected { Object (component, root, ...) } to have a length of 1 but got 2
      at Context.<anonymous> (ReactWrapper-spec.jsx:397:46)

[aweary]

Seems like we have to choose between these two behaviors. I would say that returning 2 is the correct behavior as you do have two instances where role is set to "button", but I know that's probably not what people would expect.

[blainekasten]

Yeah, i'm with @aweary. I feel like that makes the most sense. If .debug printed out multiple items with a prop, but a selector doesn't find each of them, I'd feel mislead.

I feel like we should probably get some input from @ljharb / @lelandrichardson, but if we do take this, it seems like it should probably be a major change as it might break some peoples tests.

[evisong]

Hi, team,

Is there any updates on this?

Thanks.

[ljharb]

If this would make existing tests that pass start to fail, then I think we need to have examples of those tests in master first - such that this PR has to change them to pass, making it semver-major.

[kcarnold]

This bit me in writing one of my first Enzyme tests: I was expecting to find a child React node via a prop selector, and nothing matched :( This in spite of the documentation saying "Enzyme allows you to find components and nodes based on a subset of their properties".

Newbie thought process: I expect wrapper.find(query) to find "things" that match query. Since React encourages me to think of components like just slightly special nodes, I expect "things" to include both nodes and components. If I happen to have both a component and a node that have some property, I might not realize that my selector is going to match both of them, and when my test fails because it accidentally matched two elements I'd puzzle over it for a while, but I'd eventually realize what's going on without digging through the Enzyme testsuite (which is, uh, what I did...). I'd then probably try wrapper.findComponent or wrapper.findNode if I needed to be more specific. I thus propose adding those.

(Regarding the number of items matched: since I'm used to querySelector vs querySelectorAll, I excepted find to return the first thing and findAll to give all of them. But I guess the design follows jquery.)

[ljharb]

I don't think this is relevant any more now that we're on v3. Can someone confirm?

[aweary]

@ljharb that's correct, this isn't relevant. We don't use isDOMComponent anymore. Thanks for doing this @travisperson, sorry we didn't get it in!