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

Enzyme 3: Mount rendering finds the same element twice with React.cloneElement #1253

Closed
nithinpeter opened this issue Oct 12, 2017 · 33 comments

Comments

@nithinpeter
Copy link

nithinpeter commented Oct 12, 2017

If I clone an element and try to find the same element after mount rendering, Enzyme finds the element twice.

Please see my component and test below:

Component
export const Dummy: React.StatelessComponent<DummyProps> = (
    props: BadgeProps,
) => {
    const { children, id } = props;

    const clonedChildren = React.Children.map(children, (child: any) => {
        return React.cloneElement(child);
    });

    return <div id={id}>{clonedChildren}</div>;
};
Test
import { mount } from 'enzyme';
import * as React from 'react';

import { Dummy } from 'path/to/Dummy';

describe('Dummycomponent test suite', () => {

    it('renders a Dummy', () => {

        const wrapper = mount(
            <div>
                <Dummy id="find-me"/>
            </div>,
        );

        expect(wrapper.find('#find-me').length).toBe(1);
    });
});
@ljharb
Copy link
Member

ljharb commented Oct 12, 2017

In this case, it's because it's finding both Dummy, and the div. Try wrapper.hostNodes().find()?

@nithinpeter
Copy link
Author

nithinpeter commented Oct 12, 2017

I don't think so. My result is the same even if my wrapper is

const wrapper = mount(
            <div>
                <Dummy id="find-me"/>
            </div>,
        );

Sorry, should have given a clearer example. Updated the original example.

@ljharb
Copy link
Member

ljharb commented Oct 12, 2017

Maybe I'm not being clear. Try wrapper.debug(), and you'll see two nodes with an id of "find-me".

@nithinpeter
Copy link
Author

wrapper.debug() gives me this.

    <div>
      <Component id="find-me">
        <div id="find-me" />
      </Component>
    </div>

Now I can see why is it finding two nodes. wrapper.hostNodes().debug() also gives me the same result as above.

@ljharb
Copy link
Member

ljharb commented Oct 12, 2017

Hmm - wrapper.hostNodes() should restrict it to the divs; are you saying wrapper.hostNodes().find('#find-me').length is not 1?

@nithinpeter
Copy link
Author

Yes.

@ljharb
Copy link
Member

ljharb commented Oct 12, 2017

Just to confirm: what version of enzyme, react, and adapter are you using? Can you upgrade them all to the latest (if they're not already there) and try again?

@nithinpeter
Copy link
Author

I'm on react@16.0.0, enzyme@3.1.0 and enzyme-adapter-react-16@1.0.0.

@ljharb
Copy link
Member

ljharb commented Oct 12, 2017

In that case, I'd say it's a bug in hostNodes. Can you try with shallow and see if hostNodes functions properly there?

@nithinpeter
Copy link
Author

console.log(wrapper.hostNodes().debug()); after shallow rendering the gives expected output.

    <div>
      <Component id="find-me" />
    </div>

@coding-watermelon
Copy link

Is it possible to prioritize this issue? From my point of view it is a blocker for existing projects that want to upgrade to react16

@newnewnet
Copy link

I also have Mount Rendering problem that render component twice.

@ljharb
Copy link
Member

ljharb commented Oct 27, 2017

@newnewnet can you file a new issue for that?

@coding-watermelon
Copy link

Any updates on this issue?

@ljharb
Copy link
Member

ljharb commented Nov 7, 2017

If there were updates, they'd be posted here. PRs are always welcome.

@JordanLittell
Copy link

JordanLittell commented Dec 8, 2017

It seems after reading the thread here that the bug has nothing to do with the title of the issue but rather the behavior of hostNodes. In particular, that given nithinpeter's setup, wrapper.hostNodes().find('#find-me') finds the react component and the regular div with ids of "find-me" rather than just the regular divs themselves.

I am not sure this is a bug. Wouldn't this be expected behavior? Based on my limited knowledge of the codebase, I would expect wrapper.find('#find-me').hostNodes() to return 1 because hostNodes() would be filtering against the nodes that have an id of find-me. So it would discard the React Component <Dummy id="find-me"/>. However, wrapper.hostNodes().find('#find-me') would call hostNodes on the root div, which would just return the wrapped root div. So calling wrapper.hostNodes().find('#find-me') would traverse the tree from the rendered root div (which <Dummy id="find-me"/> and it's children are nested in) and return the nodes that have an id of "find-me".

If this is not expected behavior I imagine it would be an enhancement of some kind.

@ljharb
Copy link
Member

ljharb commented Dec 8, 2017

Yes, I think that is expected behavior. You need .hostNodes() every time you want to filter out non-host-nodes; it doesn't set any kind of "host-node-only mode".

@nithinpeter what happens if you try wrapper.hostNodes().find('#find-me').debug()?

@besh
Copy link

besh commented Jan 9, 2018

Chiming in that I'm experiencing a similar issue, but hostNodes returns nothing for me.

import React from 'react';
import { Form } from 'components/views';
import { mount } from 'enzyme';

let wrapper;
let onSubmitSpy;

beforeEach(() => {
    onSubmitSpy = sinon.spy();
    wrapper = mount(
      <Form id="form" onSubmit={onSubmitSpy} className={formClass}>
        <input
          type="text"
          id="text_field"
          name="text_field"
        />

        <select id="select" name="select">
          <option value="option1">option1</option>
          <option value="option2">option2</option>
        </select>

        <input
          id="radio1"
          name="radio"
          type="radio"
          value="1"
        />
        <input
          id="radio2"
          name="radio"
          type="radio"
          value="2"
          defaultChecked
        />

        <button type="submit" id="submit_button"></button>
      </Form>
    );
});

Without hostNodes

console.log(wrapper.debug()); 

Results in

<Form id="form" onSubmit={[Function: proxy]} className="form_class_name">
  <form id="form" className="form_class_name" onSubmit={[Function]}>
    <input type="text" id="text_field" name="text_field" />
    <select id="select" name="select">
      <option value="option1">
        option1
      </option>
      <option value="option2">
        option2
      </option>
    </select>
    <input id="radio1" name="radio" type="radio" value="1" />
    <input id="radio2" name="radio" type="radio" value="2" defaultChecked={true} />
    <button type="submit" id="submit_button" />
  </form>
</Form>

With hostNodes

console.log(wrapper.hostNodes().debug()); 

Results in

''

I'm guessing that returning an empty string is not the expected behavior here?

@ljharb
Copy link
Member

ljharb commented Jan 10, 2018

@hankthewhale in fact this is expected; wrapper is only a single node - the Form. .hostNodes() filters it out, leaving an empty collection. You can verify this with wrapper.length and wrapper.hostNodes().length.

If you instead try wrapper.children().hostNodes().debug() I think you'll get what you expect.

@ljharb
Copy link
Member

ljharb commented Jan 10, 2018

This seems addressed, so I'm going to close it; happy to reopen if something actionable comes up.

@ljharb ljharb closed this as completed Jan 10, 2018
@besh
Copy link

besh commented Jan 11, 2018

@ljharb good call. I did see exactly that and it totally makes sense.

simeg added a commit to arqex/react-datetime that referenced this issue Feb 11, 2018
@BrennerSpear
Copy link

This behavior seems non-optimal. As you could previous to Enzyme 3, you should be able to do something like:
wrapper.find('#cancel_confirmtion_btn').simulate('click'); with a mounted component, and it to find only one div with the id cancle_confirm_btn.

What is the value of rendering the React Component along with the divs?

@ljharb
Copy link
Member

ljharb commented Feb 23, 2018

@BrennerSpear wrapper.find('#whatever').hostNodes().simulate('click').

The value is that the tree is consistent between mount and shallow, and provides access to both the component as well as the nodes it renders.

@BrennerSpear
Copy link

Got it. Thank you. And thank you for the example.

I'm trying to think of when you would want the React Components to also be rendered to test against them. I guess if you're dynamically rendering what is to be passed into them and would like to check against that rather than the actual divs..

@ljharb
Copy link
Member

ljharb commented Feb 23, 2018

That, as well as if you want to be able to access instance methods for any particular reason, like if you want to forcibly setState somewhere inside the tree.

@iorrah
Copy link

iorrah commented Sep 13, 2018

The following worked for me 👍

it('should contain an element called #element-id', () => {
  const { wrapper } = setupMount();
  expect(wrapper.find('#element-id').hostNodes()).toHaveLength(1);
});

@chrisrymer
Copy link

I just hit up against this, it seems you can work around this to get the HTML element by being specific about the selector.

wrapper.find('input.Myclass') as opposed to wrapper.find('.Myclass')

@ljharb
Copy link
Member

ljharb commented Sep 26, 2018

@chrisrymer alternatively, wrapper.hostNodes().find('.Myclass')

@guilherme6191
Copy link

@ljharb awesome work responding to the community!

Is it possible to make a patch in setupTests.js or somewhere else to implement hostNodes() for all find usage? I wonder that because I have a huge codebase with lots of tests that I want to upgrade to react 16.

Just so you know, about the issues with actual DOM elements and React Elements when upgrading. For me, wrapper.find('.Myclass').hostNodes() usually works but wrapper.hostNodes().find('.Myclass') returns 0 elements.

Thanks

@ljharb
Copy link
Member

ljharb commented Oct 2, 2018

No, it's not, and that would be a poor choice because it's too magic and implicit. I'd suggest writing a codemod that changes your entire codebase all at once (also, definitely upgrade to enzyme 3 and get everything passing, before upgrading your react version)

@guilherme6191
Copy link

guilherme6191 commented Oct 2, 2018

@ljharb you mean upgrade only enzyme and use React 15 adapter, and then upgrade React?

And also, you suggest the codemode to find all find occurrences and use hostNodes with it?

Thanks for the quick reply.

@ljharb
Copy link
Member

ljharb commented Oct 2, 2018

@guilherme6191 yes, precisely that - get enzyme 3 working on React 15 with the appropriate adapter first, before trying to upgrade React.

For the codemod, yes, that's probably the simplest approach - at least, all find occurrences that are passing in a selector (a string that's either a selector or an html element name).

@quantuminformation
Copy link

hostNodes fixed it for me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests