[Improved Fiber Support] Support Portals properly #1150

lelandrichardson · 2017-09-26T19:08:23Z

I am not quite sure if these are supported yet or not. We need to add some tests. I also wonder if there are any APIs that might need to get created to assert on this or something?

gziolo · 2017-09-29T09:13:37Z

I can confirm that Portals don't work with Enzyme 3.0.0 (+ adapter for React 16), React 16.0.0 and Jest 21.2.0. See related test: https://github.com/WordPress/gutenberg/pull/2813/files#diff-6cf726110898b8679af57302917acb5dR279.

ezhlobo · 2017-10-09T10:32:29Z

I can be wrong but it looks like an issue in react-test-renderer.

I tried this via jest and CRA:

const Test = () => (
  <div>
    <h1>Hello</h1>

    <Portal>Outside message</Portal>
  </div>
)

const tree = renderer.create(<Test />).toJSON()

Where Portal returns ReactDOM.createPortal(...).

The error message is

TypeError: parentInstance.children.indexOf is not a function

      at appendChild (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7778:39)
      at commitPlacement (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:5200:13)
      at commitAllHostEffects (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6196:13)
      at HTMLUnknownElement.callCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2210:14)
      at invokeEventListeners (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:219:27)
      at HTMLUnknownElementImpl._dispatch (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:126:9)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/nodes/HTMLElement-impl.js:36:27)
      at HTMLUnknownElement.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:61:35)
      at Object.invokeGuardedCallbackDev (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2249:16)
      at invokeGuardedCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2106:29)
      at commitAllWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6332:9)
      at workLoop (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6643:13)
      at HTMLUnknownElement.callCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2210:14)
      at invokeEventListeners (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:219:27)
      at HTMLUnknownElementImpl._dispatch (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:126:9)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
      at HTMLUnknownElementImpl.dispatchEvent (node_modules/jsdom/lib/jsdom/living/nodes/HTMLElement-impl.js:36:27)
      at HTMLUnknownElement.dispatchEvent (node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:61:35)
      at Object.invokeGuardedCallbackDev (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2249:16)
      at invokeGuardedCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:2106:29)
      at performWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6756:7)
      at scheduleUpdateImpl (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7141:19)
      at scheduleUpdate (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7080:12)
      at scheduleTopLevelUpdate (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7351:5)
      at Object.updateContainer (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7381:7)
      at Object.create (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:8186:18)
      at Object.fit (src/lib/tests/forms.js:25:48)
      at process._tickCallback (internal/process/next_tick.js:103:7)

lukeapage · 2017-10-12T17:37:59Z

There may well also be an issue with react-test-renderer, but the first error I get is inside the react 16 renderer adapter:

tag type of 4 is not recognized.

kellyrmilligan · 2017-10-13T13:48:40Z

I don't know exactly if this is correct, but if I add
var Portal = 4;
and duplicate the switch case for classComponent, and match against the Portal var:

case Portal:
      return {
        nodeType: 'class',
        type: node.type,
        props: (0, _object2['default'])({}, node.memoizedProps),
        key: node.key,
        ref: node.ref,
        instance: node.stateNode,
        rendered: childrenToTree(node.child)
      };

my basic tests pass. I am not familiar at all with what should be returned from this switch block though...

I am using latest version of react-modal in my app and with the react upgrade this issue surfaced in my tests. They are indeed in 3.0.0 using the new portal component if react 16 is beiing used to render it.

mayacode · 2017-10-17T15:17:53Z

Hi there,

any update?

I just switched to react 16 and my tests are failing...

Enzyme Internal Error: unknown node with tag 4

      at toTree (node_modules/enzyme-adapter-react-16/build/ReactSixteenAdapter.js:147:13)

I would appreciate any hint, how to make them work again...

ljharb · 2017-10-17T15:53:25Z

@mayacode did you start using Portals as part of your switch? A react 15 codebase switched directly to 16 shouldn't be running into that error.

kellyrmilligan · 2017-10-17T15:55:00Z

I ran into the issue after upgrading react-modal, which does use portals if they are available

mayacode · 2017-10-17T16:20:58Z

@ljharb I upgraded react (from 15.4) and react-bootstrap (in the app is react-bootstrap's Modal in use, and I assume it uses portals, if available). In the end, I made everything else working, just tests are failing with Enzyme Internal Error: unknown node with tag 4...

ljharb · 2017-10-17T16:31:15Z

thanks; if react-bootstrap uses Portals in its latest version, that would indeed trigger this code path.

TzviPM · 2017-10-17T20:12:33Z

@ezhlobo, did you mean to use ReactDom.createPortal instead of React.createPortal?

ezhlobo · 2017-10-17T20:59:47Z

@omniroot thank you for noticing this. Yeah, I meant ReactDOM, I'll update my previous message.

FezVrasta · 2017-10-18T13:44:03Z

May one of the core contributors take a look at this PR? #1263

It should address this issue, but the PR owner needs help with the tests.

mayacode · 2017-10-30T13:32:31Z

Any update for all waiting people? I needed to comment a lot of tests... my coverage dropped down ;(

oliviertassinari · 2017-10-30T13:36:13Z

@mayacode, you can always mock the behavior of ReactDOM.createPortal to be an identity function. At least, I ended-up mocking the internal Portal of Material-UI to migrate all the tests to react@16 without losing test coverage.

ljharb · 2017-10-30T20:38:45Z

I'm confused; testing Portals with enzyme and React 16 has never been possible; how could you have working tests that use them that need commenting out?

kellyrmilligan · 2017-10-30T22:30:07Z

Not exactly sure, but with react 15 and enzyme < 3, tests still worked, and when upgraded they totally fail. I wasn’t testing the portal itself per se, but a component that had one still was able to be rendered. This started happening to my project when I upgraded to latest enzyme and react 16

ljharb · 2017-10-30T22:34:38Z

@kellyrmilligan right but with latest enzyme and react 15, do you still see a problem?

Enzyme doesn't yet support Portals with React 16.

kellyrmilligan · 2017-10-30T22:51:41Z

I’ll give it a go. But I think we’re all just wanting this to at least not fail on that node type? This is blocking my upgrade path for the time being.

kellyrmilligan · 2017-10-31T16:52:13Z

correct, I branched off and just upgraded enzyme to latest, and this issue doesn't occur. again, i'm not sure here, but seems like a lot of folks would want portals to at least just not fail. there's even a PR for this issue.

#1263

ljharb · 2017-10-31T19:08:58Z

Of course, it should be fixed :-) just wanted to clarify that the issue only blocks upgrading to React 16, not upgrading to enzyme 3.

mayacode · 2017-11-01T09:56:18Z

@ljharb I have tests which are using react-bootstrap Modals, which are using react-overlays, which is using portals (if available). In a moment of upgrading react and react-bootstrap all Modal related tests started to fail. It blocks upgrading to react 16.

ljharb · 2017-11-01T16:46:00Z

gotcha; it seems like the combination of enzyme not supporting Portals, and react-overlays magically (instead of explicitly) using Portals, is the blocker.

- add Portal support (enzymejs#1150, enzymejs#1263, enzymejs#1338, enzymejs#1345)

reyronald · 2018-03-07T19:02:10Z

For the moment I've been mocking portals in the top of the test file like this:

jest.mock('rc-util/lib/Portal')

Obviously the rc-util package is needed for this.

lelandrichardson added enhancement help wanted labels Sep 26, 2017

ljharb mentioned this issue Sep 27, 2017

Adapter React 16 does not support new Portal node types #1164

Closed

gziolo mentioned this issue Sep 28, 2017

Upgrade some dependencies: React 16, Enzyme 3 and Jest WordPress/gutenberg#2813

Closed

4 tasks

flakeparadigm mentioned this issue Oct 5, 2017

Support Portal components #252

Closed

ljharb added Fiber Support package: react adapter: 16 labels Oct 12, 2017

didia mentioned this issue Oct 16, 2017

Make the app responsive next-africa/diaspoship-web#22

Merged

2 tasks

kellyrmilligan mentioned this issue Oct 18, 2017

Add documentation for Enzyme 3 integration facebook/create-react-app#3286

Merged

tajo mentioned this issue Oct 19, 2017

Enzyme doesn't support Portals tajo/react-portal#171

Closed

oliviertassinari mentioned this issue Oct 23, 2017

Migrate all dependencies to react@16 mui/material-ui#8801

Closed

vietnogi mentioned this issue Nov 3, 2017

TypeError: parentInstance.children.indexOf is not a function reactjs/react-modal#553

Closed

comfroels mentioned this issue Nov 3, 2017

Using React 16 portals reactjs/react-modal#554

Closed

mzabriskie mentioned this issue Nov 10, 2017

Adding support for ReactDOM.createPortal #1345

Merged

ljharb closed this as completed in #1345 Nov 15, 2017

ljharb added a commit to ljharb/enzyme that referenced this issue Nov 15, 2017

[enzyme-adapter-react-16] v1.1.0

3ebc14a

- add Portal support (enzymejs#1150, enzymejs#1263, enzymejs#1338, enzymejs#1345)

RWOverdijk mentioned this issue Nov 20, 2017

[test] Modal snapshot test fail "TypeError: parentInstance.children.indexOf is not a function" mui/material-ui#9243

Closed

1 task

UselessPickles mentioned this issue Dec 3, 2017

Update Portal to use React 16 Portals for simpler unit testing with Enzyme. palantir/blueprint#1860

Closed

arizzitano mentioned this issue Dec 11, 2017

feat(modal): refactor modal to use portals openedx/paragon#95

Closed

1 task

kvolkovich-sc mentioned this issue Dec 12, 2017

Skip some tests after drop React v 0.14.x OpusCapita/react-dates#41

Open

jquense pushed a commit to jquense/enzyme that referenced this issue Feb 23, 2018

[enzyme-adapter-react-16] v1.1.0

804913b

- add Portal support (enzymejs#1150, enzymejs#1263, enzymejs#1338, enzymejs#1345)

ameliabradley mentioned this issue Aug 28, 2019

Modal can't be tested with enzyme reactstrap/reactstrap#1322

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Improved Fiber Support] Support Portals properly #1150

[Improved Fiber Support] Support Portals properly #1150

lelandrichardson commented Sep 26, 2017

gziolo commented Sep 29, 2017 •

edited

Loading

ezhlobo commented Oct 9, 2017 •

edited

Loading

lukeapage commented Oct 12, 2017

kellyrmilligan commented Oct 13, 2017 •

edited

Loading

mayacode commented Oct 17, 2017 •

edited

Loading

ljharb commented Oct 17, 2017

kellyrmilligan commented Oct 17, 2017

mayacode commented Oct 17, 2017 •

edited

Loading

ljharb commented Oct 17, 2017

TzviPM commented Oct 17, 2017

ezhlobo commented Oct 17, 2017

FezVrasta commented Oct 18, 2017

mayacode commented Oct 30, 2017

oliviertassinari commented Oct 30, 2017

ljharb commented Oct 30, 2017

kellyrmilligan commented Oct 30, 2017

ljharb commented Oct 30, 2017

kellyrmilligan commented Oct 30, 2017

kellyrmilligan commented Oct 31, 2017

ljharb commented Oct 31, 2017

mayacode commented Nov 1, 2017

ljharb commented Nov 1, 2017

reyronald commented Mar 7, 2018

[Improved Fiber Support] Support Portals properly #1150

[Improved Fiber Support] Support Portals properly #1150

Comments

lelandrichardson commented Sep 26, 2017

gziolo commented Sep 29, 2017 • edited Loading

ezhlobo commented Oct 9, 2017 • edited Loading

lukeapage commented Oct 12, 2017

kellyrmilligan commented Oct 13, 2017 • edited Loading

mayacode commented Oct 17, 2017 • edited Loading

ljharb commented Oct 17, 2017

kellyrmilligan commented Oct 17, 2017

mayacode commented Oct 17, 2017 • edited Loading

ljharb commented Oct 17, 2017

TzviPM commented Oct 17, 2017

ezhlobo commented Oct 17, 2017

FezVrasta commented Oct 18, 2017

mayacode commented Oct 30, 2017

oliviertassinari commented Oct 30, 2017

ljharb commented Oct 30, 2017

kellyrmilligan commented Oct 30, 2017

ljharb commented Oct 30, 2017

kellyrmilligan commented Oct 30, 2017

kellyrmilligan commented Oct 31, 2017

ljharb commented Oct 31, 2017

mayacode commented Nov 1, 2017

ljharb commented Nov 1, 2017

reyronald commented Mar 7, 2018

gziolo commented Sep 29, 2017 •

edited

Loading

ezhlobo commented Oct 9, 2017 •

edited

Loading

kellyrmilligan commented Oct 13, 2017 •

edited

Loading

mayacode commented Oct 17, 2017 •

edited

Loading

mayacode commented Oct 17, 2017 •

edited

Loading