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

Support Portal components #252

Closed
Tracked by #1553
leoselig opened this issue Mar 14, 2016 · 29 comments
Closed
Tracked by #1553

Support Portal components #252

leoselig opened this issue Mar 14, 2016 · 29 comments

Comments

@leoselig
Copy link

We are currently trying to test a component that uses Material UI's SelectField (a dropdown menu). They use their own Popover component (https://github.com/callemall/material-ui/tree/master/src/DropDownMenu) to bypass the typical overflow: hidden; issue where parts of the overlay is clipped off. Essentially, they are mounting the children (the menu items of the dropdown) into the body.
Unfortunately, enzyme cannot find them this way. E.g.

selectField = mount(
  <SelectField value={this.state.value} onChange={this.handleChange}>
    <MenuItem value={1} primaryText="Never"/>
    <MenuItem value={2} primaryText="Every Night"/>
    <MenuItem value={3} primaryText="Weeknights"/>
    <MenuItem value={4} primaryText="Weekends"/>
    <MenuItem value={5} primaryText="Weekly"/>
  </SelectField>
);
selectField.find(MenuItem).length === 0;

Are we missing any way to deal with this?

@blainekasten
Copy link
Contributor

If the react element is actually rendered outside of the parent component, then this seems out of enzyme's scope.

@leoselig
Copy link
Author

I understand it might be at the moment
But the concept of portal components (found out these have a fancy name) is typical in many situations and also what renderSubtreeIntoContainer() is aming for to simplify
I'm just curious if there is any plan or idea on how to support this in the future since it is almost impossible to write sane tests for this right now

@frontendphil
Copy link

Any news on this? I'm running in the exact same issues except that I'm not using the Material lib, but something custom build. However, the subtree is rendered with ReactDOM.unstable_renderSubtreeIntoContainer and I'm wondering if that provides any information that enzyme might use.

@blainekasten
Copy link
Contributor

I would suggest putting up your own PR. I would assume if airbnb doesn't have this issue, they probably won't prioritize doing it themselves. But they would probably accept a PR!

@CrshOverride
Copy link

I thought I'd chime in with my 👍. I'm running into this issue when trying to test a component that uses react-bootstrap's Modal Dialog.

@aweary
Copy link
Collaborator

aweary commented May 4, 2016

When you mount a component with a subtree what does the returned wrapper look like? I think this would rather difficult to implement, and potentially out of scope as @blainekasten said--at least until renderSubtreeIntoContainer is a public API (though if there was a way to implement this efficiently I'm sure a PR would be welcome).

@CrshOverride
Copy link

@aweary I'm going to see if there's something I can patch in and make a quick/easy PR that might fix the issue. I just didn't have the time earlier today.

FWIW - It looks like the React Test Utils are able to handle this case by themselves:

reactjs/react-modal#71

@aweary
Copy link
Collaborator

aweary commented May 4, 2016

@CrshOverride seems like they handle it by attached the subtree to a portal property, I think that's an implementation detail for react-modal that won't carry over to, say, Material UI. If this is going to be supported it should be able to handle any kind of component with a subtree rendered within it somewhere.

@CrshOverride
Copy link

@aweary It also looks like there's a way to utilize TestUtils to fetch the ReactElement. Once you have that, you should be able to easily instantiate a ReactWrapper around it (if I've read the constructor for ReactWrapper properly, that is). Unfortunately that really doesn't fit into the current shallow/mount/render API.

I have a feeling that nobody wants to add a top level method for fetching and wrapping rendered components from the DOM though.

@nvdbleek
Copy link

nvdbleek commented Jul 8, 2016

Is there any progress on testing portal components with enzyme?

@CrshOverride
Copy link

CrshOverride commented Jul 12, 2016

@nvdbleek In my case, I was able to take advantage of the implementation details of React Bootstrap's modal and access the portal component from there. I created a simple helper to create a ReactWrapper around that component and it's working for us so far.

@shir
Copy link

shir commented Oct 28, 2016

@CrshOverride Hi. Could you provide details of your implementation?

@mmiller42
Copy link

mmiller42 commented Dec 9, 2016

In order to get it to work, you need to attach a ref to an element -- specifically, the root node that will be transported to a new location in the DOM (i.e. the child of your portal element).

import React, { PureComponent } from 'react'
import PropTypes from 'prop-types'

export default class MyModal extends PureComponent {
  static propTypes = {
    children: PropTypes.node,
  }

  render() {
    return (
      <Portal>
        <div ref={ref => { this.modalRef = ref }}>
          {this.props.children}
        </div>
      </Portal>
    )
  }
}

You can retrieve the ref using the instance method on a mounted component. Then you can wrap that in a ReactWrapper.

import React from 'react'
import { mount, ReactWrapper } from 'enzyme'

const modal = new ReactWrapper(
  mount(
    <MyModal>
      <p>Hello world</p>
    </MyModal>
  )
    .instance()
    .modalRef,
  true
)

expect(modal.html()).to.match(/Hello world/)

@gor181
Copy link

gor181 commented Dec 22, 2016

@mmiller42 approach works.

image

Just be sure that you attach the ref to the Modal.Body or Modal.Header as attaching to Modal isn't going to work.
Example:
<Modal.Body ref={ref => (this.modalRef = ref)}>

Thanks!

@blainekasten
Copy link
Contributor

Closing this as duplicate of #535

@leoselig
Copy link
Author

@blainekasten
I'm afraid I see this issue to NOT be a duplicate of #535
#535 deals with the interaction with external mounting of nodes by custom/third-party mechanisms
However, as pointed out in the beginning of this issue, this is about compatibility with React's own and official 'ReactDOM.unstable_renderSubtreeIntoContainer()'

I believe this should be kept open unless I missed something here

@blainekasten
Copy link
Contributor

@leoselig Good point. It is technically different. I'll update the title and re-open. FWIW, We're not likely to support an unstable_ API. It could result in too much churn and difficulty for our users. Though, when Fiber is released, they have dedicated support for Portal components. With a dedicated API we'll easily be able to support (confidently) this concept. Until then, this will likely sit in waiting.

@blainekasten blainekasten reopened this Dec 29, 2016
@blainekasten blainekasten changed the title Finding component mounted "elsewhere" (e.g. Popover) Support Portal components Dec 29, 2016
@leoselig
Copy link
Author

leoselig commented Jan 2, 2017

Absolutely reasonable, did not know about portal component support in fiber - awesome!
Thanks for explaining and re-opening

vanderhoop pushed a commit to stride-nyc/remote_retro that referenced this issue May 29, 2017
  - For testing new modal confirmations, need to use a ref to access rendered modal content
    - per enzymejs/enzyme#252 (comment)
@jstray
Copy link

jstray commented Aug 11, 2017

This cost me so much time that I created a StackOverflow question with a fairly general answer, based on document.getElementsByClassName().

@blainekasten
Copy link
Contributor

The good news is React 16 will feature a Portals api. Likely meaning Enzyme should be able to hook into that API and become testable.

@robin-anil
Copy link

Since React 16 is now released, I would like to upvote this issue

@leoselig
Copy link
Author

leoselig commented Oct 3, 2017

@r-tock Did you verify that it still does not work? Since enzyme uses the official react-test-renderer I was hoping portals would work out of the box
I couldn't test it in our project yet though due to enzymejs/chai-enzyme#199

@robin-anil
Copy link

Yeah, we had to disable tests because they were failing on mount() of a portal wrapped component with a simple div underneath it

@flakeparadigm
Copy link

Supporting the React 16 Portals api in Enzyme 3 is mentioned in this much more recent issue: #1150

@UselessPickles
Copy link

UselessPickles commented Nov 30, 2017

What's the status on this? I see that enzyme-adapter-react-16 has been updated to support portals here: #1345

But even after updating all my NPM dependencies, including getting those changes in the react adapter, this issue is still not fixed. Are there more changes required in other enzyme code to properly use the new portal support in mount()?

@UselessPickles
Copy link

Looks like my problem is that I'm using Blueprintjs, and their Portal component does not yet make use of ReactDOM.createPortal. In case anyone else stumbles across this while using Blueprint, I have created an issue in their project here: palantir/blueprint#1860

@oliviertassinari
Copy link

As far as I understand the issue, it was fixed along the way. I think that we can close it.

@jstray
Copy link

jstray commented Jun 1, 2018

Agreed, no longer an issue in Enzyme 3 + React 16, should be closed.

@ashvin777

This comment has been minimized.

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