Enzyme 3.0.0 "find()" queries not just DOM elements

[FezVrasta]

I just switched to Enzyme 3.0.0 and React 16 and I noticed that some of my tests are failing because Enzyme .find is finding more than one element with the given ID.

// App.js
function Test(props) {
  return <div {...props} />
}

export default function App() {
  return <Test id="foobar" />;
}

// App.test.js

it('finds a single DOM node with the given ID', () => {
  expect(mount(<App />).find('#foobar').length).toBe(1); // fails saying that it found 2 elements
});

Repro repository here:
https://github.com/FezVrasta/enzyme-id-bug-report

Just run yarn && yarn test to reproduce.

I don't think this is the expected behavior because:

1. It didn't work this way previously
2. The .find method is now looking for DOM Elements AND React elements with the given ID instead of just looking at the DOM elements.

If this is the expected behavior, how can I obtain the previous result? Is there a findDOMNode or something similar?
Or maybe an helper function that strips from the selection all the "non DOM" elements?

Thanks!

[green-arrow]

I'm seeing this issue as well. After upgrading, some of our .find queries that were looking for elements by name ended up "breaking".

// Returned 1 element originally
// Now returns 5, one for each React component with the given name
component.find('[name="close-drawer"]');

Thanks for all the hard work that you all put into this release!

[aweary]

Thanks for the report @FezVrasta. find not matching components was an issue that was reported and discussed a few times (#582, #534). It's a hard issue: either we restrict find to DOM nodes which means you can't use it to find components, or we allow both and risk duplicate results.

I think the second approach of risking duplicates is the more sustainable one, since you can always use a more specific selector to filter your results. For example, instead of #foobar you can do div#foobar.

What do you think @lelandrichardson @ljharb? Are we OK with this behavior in v3? If so we should add a note to the migration guide.

[FezVrasta]

Thanks for the reply!

I'm okay with this, but I'd love a way to match only DOM Nodes, maybe a chained method?

wrapper.find('#foobar').filterDOMNodes()

Or anything similar to be able to mimic the old behavior.

[lelandrichardson]

@FezVrasta you can do wrapper.find('#foobar').filter(n => typeof n.type() === 'string')

You could also do...

import { ReactWrapper } from 'enzyme';
ReactWrapper.prototype.hostNodes = function() {
  return this.filter(n => typeof n.type() === 'string');
};

// ...

wrapper.find('#foobar').hostNodes();

[lelandrichardson]

Also, I would love it if someone memorialized this change in the migration guide!

[ljharb]

Maybe we should add .hostNodes rather than suggesting people monkeypatch enyzme tho :-p

[lelandrichardson]

someone should throw up a PR :) I'm not against the idea.

[elyobo]

Using .render().find() will also work to restrict it to DOM nodes, but may or may not be OK depending on what else you need to test. For my purposes it worked fine, but tweaking my selectors to selector on node type as well as class (as suggeted in #1174 (comment)) also worked fine.

[pfhayes]

I made #1185 without realizing it was a duplicate of this issue. To merge the discussions, it appears that .find() queries DOM elements only for shallow, but all nodes for mount. Which behaviour is intended, and are they supposed to differ?

[ljharb]

It's fine that they differ; but are you sure that's the case for shallow? I believe shallow(() => <div><Foo className="foo" /><div className="foo" /></div>).find('.foo').length should return 2, for any Foo.

[pfhayes]

it('mounts', () => {
      const Foo = (props) => <div className={props.className}/>;
      expect(shallow(<div><Foo className="foo" /><div className="foo" /></div>).find('.foo').length).toEqual(2);
      expect(mount(<div><Foo className="foo" /><div className="foo" /></div>).find('.foo').length).toEqual(2);
});

fails on the mount (expected 2, got 3) clause but not the shallow clause

This is certainly surprising, and made more surprising by the fact that AFAICT this was not the case in enzyme 2 and I was not able to find this documented anywhere

So once .hostNodes() is added should we just prepend that to all .find() calls where we intend to only query the DOM?

[ljharb]

Yes.

However, @aweary - do you think it makes sense to apply the selector logic to non-host-nodes? I think that @pfhayes might be right that in enzyme 2, selectors only applied to host nodes.

[aweary]

@ljharb yeah, @pfhayes is correct. Enzyme 2 would only apply to host nodes. I think it makes more sense to apply the selector logic to all nodes. If it applies to all nodes you can use hostNodes() or more specific queries like div.foo to target some subset of host nodes, while still supporting queries that target composite nodes. The opposite wouldn't be true if we only supported host nodes by default.

[pfhayes]

So to clarify: it is intended behaviour that .find in Enzyme 2's mount only queried host nodes and and .find in Enzyme 3's mount queries all nodes.

What is the decision on .find in Enzyme 3's shallow? Should it also query all nodes?

[ljharb]

I would expect, for consistency, that it would also query all nodes. Can you file a separate issue for that?

[nathan-charrois]

If anyone isn't clear on why .find().length returns 2 when previously it returned 1, try using .debug() on the found element(s) to see exactly what's going on.

Using .html() on the mounted component wont show what enzyme is trying to parse.

Seeing that the host node is now included I only had to change my selector. I am in favour of something like .filterDOMNodes() but native filter seems fine in the rare cases that I would need it.

[ljharb]

@NathanCH v3.1 has .hostNodes for this purpose.

[dschinkel]

I know this isn't related directly to your code but I suggest to everyone to stop using element names directly on your find()s. Use css feature markers. Add a className='ft-some-feature' on it and search on that. Makes your life easier and decouples your tests from the implementation, implementation being whatever element your test is testing. If your tests are bound to finding element names you end up having to import that element plus your tests are coupled to that element name. If someone ever changes that element name your tests will break for the wrong reasons...you don't want your tests to break just because of a rename. People need to stop coupling their tests to the DOM or to Element names as much as possible.

[dschinkel]

What is the decision on .find in Enzyme 3's shallow? Should it also query all nodes?

are you saying shallow should act like mount when you say all nodes? that wouldn't make sense. We need to dive() if we want to go x levels deep.

[ljharb]

The question is about a component that contains both DOM and custom components in its shallow render.

Separately, I do not recommend using CSS classNames - this is an abuse of CSS. If you want a feature marker, use data- attributes, but it's much better to locate specific Components, by constructor/function instead of by name.

[dschinkel]

@ljharb

hmm I know a lot of places that use feature markers..not sure how it's an abuse of CSS. Are you saying it's cluttering CSS? I'm not using css classes that are defined as actual styles.... they're just classes used for tests, it's pretty common. I'm not using classes that have styling in them, they're not even in a stylesheet. In this case it's totally fine. I've also had problems with data- attributes and do not want to couple my tests to magic.

I'd agree it's an abuse if you were reusing styling based css classes bug that's not what I'm referring to.

it's much better to locate specific Components, by constructor/function instead of by name.

can you elaborate?

[dschinkel]

I could use data attributes, migh be cleaner in some people's view but others might argue something else such as that I'm abusing data attributes :). it’s 6 of one and 1/2 dozen of another on this one, nobody's gonna win this argument but anyway Thanks @ljharb for bring up the thoughts...good to know. And thanks for answering the initial question of mine above.

[ljharb]

It's an abuse of className; feature markers are fine in general as data attributes - the sole purpose of data attributes is for "site data that doesn't fit into the existing semantic attributes", so it's actually conceptually invalid to use classNames for anything other than styling.

Regardless, with React and enzyme, feature markers are obsolete - you can wrapper.find(RenderedComponent) and assert on what that renders in the separate shallow tests for RenderedComponent.

[hon2a]

This should really be added to migration docs, as it clearly is a common migration issue.

---

@ljharb className has no inherent ties to styling. Its purpose is to categorize elements by their meaning, purpose, or "nature", as the spec puts it:

The attribute, if specified, must have a value that is a set of space-separated tokens representing the various classes that the element belongs to.
There are no additional restrictions on the tokens authors can use in the class attribute, but authors are encouraged to use values that describe the nature of the content, rather than values that describe the desired presentation of the content.

Assuming that classes are tied only to CSS is a common misconception. That said, I agree with the rest of your recommendation :)

[ljharb]

@hon2a the nature of an element is also something that the component should control, and should not be broadly determinable by parents.