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

[Improved Fiber Support] Support Array-rendering components #1149

Open
Tracked by #1553
lelandrichardson opened this issue Sep 26, 2017 · 16 comments
Open
Tracked by #1553

[Improved Fiber Support] Support Array-rendering components #1149

lelandrichardson opened this issue Sep 26, 2017 · 16 comments

Comments

@lelandrichardson
Copy link
Collaborator

I believe we still have a little bit of work to do on this front.

@nickserv
Copy link

nickserv commented Sep 28, 2017

I noticed that all my .find() calls fail in components that I've migrated to Array returns. It seems like I need to avoid it entirely until this is supported in Enzyme.

I found this a little confusing since at first I wasn't sure if I set up my Array returns properly, some sort of temporary error message from Enzyme about this being unsupported would be helpful.

@aamirafridi
Copy link

👍🏼
I have to wrap the array of jsx into a div just to pass the tests :(

@udivankin
Copy link

udivankin commented Dec 4, 2017

FYI wrapping code with <Fragment /> that was implemented in React v16.2 seems to do the trick.

@zerubeus
Copy link

@udivankin Nice but we have to migrate to React 16.2 to be able to use this new API :/

@FezVrasta
Copy link
Contributor

FezVrasta commented Dec 21, 2017

And Fragment is not supported by Enzyme, anyway

@tkrotoff
Copy link

tkrotoff commented Feb 7, 2018

@udivankin

<Fragment /> seems to do the trick

Does not work

@JakobJingleheimer
Copy link

<React.Fragment> as a wrapper is working for me (React 16.2.0 & Enzyme 3.3.0):

const wrapper = shallow(<MyComponent />);

console.log(
    wrapper.debug(),
    wrapper.children().debug(),
);

log output:

<Symbol(react.fragment)>
  <PaymentIcon amount="$10.00" displayName="John Jingleheimer" type="card" size="huge" />
  <Header as="h4" content={[undefined]} />
</Symbol(react.fragment)>

<PaymentIcon amount="$10.00" displayName="John Jingleheimer" type="card" size="huge" />
<Header as="h4" content={[undefined]} />

Which is exactly what I expect.

@brandonduff
Copy link

brandonduff commented Mar 30, 2018

@jshado1 In your example, wrapper.text() and wrapper.html() throw this error:

Error: Trying to get host node of an array

This issue is also relevant #1178

EDIT: Actually, this works with shallow but not with mount.

@knightjdr
Copy link

Just to clarify for people like me, if using at least React V16.2, <Fragment> will work if placed in your returned code, but not if you wrap your component with it in your test.

i.e. the following does not work (using enzyme 3.3.0):

import React, { Fragment } from 'react';
import { shallow } from 'enzyme';

const MyComponent = () => ([
   <div className="first-div" key="a" />,
   <div className="second-div" key="b" />,
]);

describe('My component', () => {
   it('should render a div with class first-div', () => {
      const wrapper = shallow(
         <Fragment>
            <MyComponent />
         </Fragment>,
      );
      expect(wrapper.find('.first-div').length).toBe(1);
   });
});

Invariant Violation: ReactShallowRenderer render(): Shallow rendering works only with custom components, but the provided element type was 'symbol'.

This works:

import React, { Fragment } from 'react';
import { shallow } from 'enzyme';

const MyComponent = () => (
   <Fragment>
      <div className="first-div" />
      <div className="second-div" />
   </Fragment>
);

describe('My component', () => {
   it('should render a div with class first-div', () => {
      const wrapper = shallow(<MyComponent />);
      expect(wrapper.find('.first-div').length).toBe(1);
   });
});

@LandonSchropp
Copy link

LandonSchropp commented Aug 15, 2018

@lelandrichardson This issue has turned into a blocker for my team, and I'd love to help resolve it.

  • Would you be open to accepting a PR to fix this?
  • Is this an issue that somebody who's not familiar with the codebase can solve?
  • Could you point me in the right direction for what needs to be done?
  • Is there any documentation covering the adapter interface?
  • Is the current API for the adapters (particularly nodeToHostNode) compatible with some of the new paradigms in React?
  • Does the core library need to be updated to support this?

@ljharb
Copy link
Member

ljharb commented Aug 15, 2018

@LandonSchropp

  • yes, we'd definitely be open to PRs to address it.
  • it's always possible, but i suspect it will be difficult for someone who isn't familiar with the codebase
  • we'd need to come up with a way that allowed the React 16 adapters to support non-node renders, without baking any React-specific behavior into enzyme itself, while not breaking React < 16 adapters or non-React adapters that may be out there.
  • I don't think we have great documentation yet on adapters; a PR that adds docs for that would also be appreciated. In the meantime, looking at the existing adapters is probably your best bet.
  • It's entirely possible that the existing APIs are incompatible with the new paradigm of non-node renders; it's hard to know that until the implementation runs into it.
  • Yes, I'm reasonably sure that it would.

@LandonSchropp
Copy link

@ljharb Thanks for the quick reply!

Could you explain the purpose of nodeToHostName? My understanding of React is that it normally calls the component tree from the top down. Is the "host name" the parent component? Or is it something different? Why would you need to go in the other direction?

I'm happy to take a crack at it, but I need a little more context. I've read through some of the other adapters, but it's difficult to understand the purpose of some of the exposed methods.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

nodeToHostNode, not nodeToHostName - the purpose is to take an RST node, and return the "host node" - ie, the HTML DOM node.

@LandonSchropp
Copy link

I think I have a workable solution to this problem. I rewrote nodeToHostNode and took a recursive approach. In the case of an array, I grabbed its first child's HTML element and then stepped up a level in the DOM. This seems to work for me locally in my React project.

function nodeToHostNode(node) {

  // If the node is null, there is no host node.
  if (!node) { return null; }

  // If the node is an array, we can get to the host node through one of its elements.
  if (Array.isArray(node)) {

    // However, if the array is empty, we have no way of getting to the host node.
    if (node.length === 0) {
      throw new Error('Trying to get host node of an empty array');
    }

    // Grab the parent element. If we don't do this, we'll only get part of the rendered fragment.
    let childElement = nodeToHostNode(node[0]);
    return childElement && childElement.parentElement;
  }

  // If the component is a functional component, we can still get at the host node associated with
  // its return value.
  if (node.instance === null) {
    return nodeToHostNode(node.rendered);
  }

  // If we've made it this far, we can query ReactDOM for the host node.
  return ReactDOM.findDOMNode(node.instance);
}

Will this approach work in all required instances? Did I miss something here? How should I go about testing this?

@ljharb
Copy link
Member

ljharb commented Aug 16, 2018

I'm not sure or not :-) we'd need to add extensive test cases for both shallow and mount, for as many wrapper methods as possible.

@publicJorn
Copy link

publicJorn commented Nov 29, 2018

I don't like adding <Fragment> to my component because Enzyme has a bug is acting different than expected. For now I'm using this as a workaround (modified @knightjdr's example):

import React, { Fragment } from 'react';
import { shallow } from 'enzyme';

const classNames = ['first-div', 'second-div'];

const MyComponent = () => classNames.map((cn) => <div className={cn} />);

describe('<MyComponent />', () => {
   it('should render a div with class first-div', () => {
      const wrapper = shallow(<MyComponent />);
      expect(wrapper.at(0).hasClass('first-div')).toBe(true);
   });
});

--Edit:

Another example; You can wrap the shallow-ed component in your test and do something like this:

import React, { Fragment } from 'react';
import { shallow } from 'enzyme';

const classNames = ['first-div', 'second-div'];

const MyComponent = () => classNames.map((cn) => <div className={cn} />);

describe('<MyComponent />', () => {
   it('should render a div with class first-div', () => {
      const divs = shallow(<MyComponent />);
      const wrapper = shallow(<div>{divs}</div>);

      expect(wrapper.find('div').length).toBe(2);
   });
});

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