The Wayback Machine - https://web.archive.org/web/20220205191302/https://github.com/facebook/react/issues/11565
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

React-test-renderer: support for portal #11565

Open
alansouzati opened this issue Nov 15, 2017 · 44 comments
Open

React-test-renderer: support for portal #11565

alansouzati opened this issue Nov 15, 2017 · 44 comments

Comments

@alansouzati
Copy link
Contributor

@alansouzati alansouzati commented Nov 15, 2017

Do you want to request a feature or report a bug?

Report a bug

What is the current behavior?

This test

import React from 'react';
import { createPortal } from 'react-dom';
import renderer from 'react-test-renderer';

const Drop = () => (
  createPortal(
    <div>hello</div>,
    this.dropContainer
  )
);

test('Drop renders', () => {
  const component = renderer.create(
    <div>
      <input />
      <Drop />
    </div>
  );
  const tree = component.toJSON();
  expect(tree).toMatchSnapshot();
});

fails with

Invariant Violation: Drop(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.

This test passes if I wrap createPortal in a container.

<div>
  {createPortal(
    <div>hello</div>,
    this.dropContainer
  )}
</div>

What is the expected behavior?

The code without the parent container works fine in the browser. So it seems that I'm adding the parent div just for the test to pass. I believe react-test-renderer should support empty returns?

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Lastest

@gaearon
Copy link
Member

@gaearon gaearon commented Nov 18, 2017

Do you want to dig into why this happens?

@505aaron
Copy link

@505aaron 505aaron commented Nov 20, 2017

@gaearon I would be interested in taking a look if @alansouzati is okay with that.

@alansouzati
Copy link
Contributor Author

@alansouzati alansouzati commented Nov 21, 2017

sure. I'm ok if you want to investigate that @505aaron.
Let me know if you need help.

@esturcke
Copy link

@esturcke esturcke commented Nov 23, 2017

I've been running into problems testing portals in a CRA project with react-test-renderer as well, but with a different failure. I tried the reproduction case above, but wasn't sure how @alansouzati defined this.dropContainer. I tried:

const dropContainer = document.body.appendChild(document.createElement("div"))
const Drop = () => createPortal(<div>hello</div>, dropContainer)

The error I'm getting is

 FAIL  src/Drop.spec.js
  ● Drop renders

    TypeError: parentInstance.children.indexOf is not a function

      at appendChild (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7183:39)
      at commitPlacement (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:4913:13)
      at commitAllHostEffects (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:5680:13)
      at HTMLUnknownElement.callCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1604: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:1643:16)
      at invokeGuardedCallback (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:1500:29)
      at commitRoot (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:5800:9)
      at performWorkOnRoot (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6767:42)
      at performWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6716:7)
      at requestWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6625:7)
      at scheduleWorkImpl (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6508:11)
      at scheduleWork (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6470:12)
      at scheduleTopLevelUpdate (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6941:5)
      at Object.updateContainer (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:6979:7)
      at Object.create (node_modules/react-test-renderer/cjs/react-test-renderer.development.js:7599:18)
      at Object.<anonymous>.test (src/Drop.spec.js:8:49)
          at new Promise (<anonymous>)
      at Promise.resolve.then.el (node_modules/p-map/index.js:46:16)
          at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:188:7)

  ✕ Drop renders (7ms)

Is this a separate issue or am I doing something wrong?

@505aaron
Copy link

@505aaron 505aaron commented Nov 30, 2017

Hello @esturcke, I encountered the same issue. I hit the same error as you. I also tried using a mock ref as documented https://reactjs.org/docs/test-renderer.html#ideas.

Test renderer obviously needs to remain separate from react-dom. I think mocking createPortal or extending the test renderer to make it easier to mock createPortal similar to ref mocking.

Sorry for the delay, I was on vacation.

@505aaron
Copy link

@505aaron 505aaron commented Dec 1, 2017

@esturcke @alansouzati Here is a gist that has a sample mock and snapshot test:
https://gist.github.com/505aaron/d5efc2ecc622306cbcc6d3e9c1d7ee9f

@gaearon probably has a better idea/plan for supporting createPortal in react-test-renderer.

@yossijacob
Copy link

@yossijacob yossijacob commented Dec 14, 2017

so @esturcke ,what did you do ?

@esturcke
Copy link

@esturcke esturcke commented Dec 14, 2017

Hi @yossijacob. I've sort of been pushing off trying to deal with it and moved on to working on other things in the hopes that @gaearon and others might point out what I'm doing wrong or add support to react-test-renderer.

@malstoun
Copy link

@malstoun malstoun commented Dec 15, 2017

We've encountered the same problem with parentInstance.children.indexOf is not a function error.

I dig a little and found, that parentInstance.children in case of portal is an instance of HTMLCollection, but test-renderer expects array of Instance type or TextInstance type in appendChild method.

I can work on this, but I need a little guidance @gaearon

@zxydaisy
Copy link

@zxydaisy zxydaisy commented Jan 22, 2018

i have the same problem @gaearon

@kairiruutel
Copy link

@kairiruutel kairiruutel commented Jan 22, 2018

I have same problem with snapshot tests for components which use ReactDOM.createPortal. I am using latest version of react-test-renderer, are there any updates related to the error (TypeError: parentInstance.children.indexOf is not a function) @gaearon ?

@diasbruno
Copy link

@diasbruno diasbruno commented Feb 27, 2018

Hi, I was investigating this issue and here is what I found.

As @malstoun pointed out, the reason is an incompatibility between createPortal() and react-test-renderer (in case your problem isparentInstance.children.indexOf is not a function).

A workaround is to tricking both APIs to accept the "new container" (this is very hacky).

const ELEMENT_TYPE = 1;
const dropContainer = { children: [], nodeType: ELEMENT_TYPE };
// if necessary, you can include methods like `appendChild` to this object.

For anyone investigating this...nodeType will be checked by the isValidElement() on createPortal() and the children can be used by the react-dom-renderer internals.

#12289 is a temporary fix/helper. It will export the toJSON from react-test-renderer to be used to write the test.

const modalTree = toJSON(dropContainer.children[0]);
expect(modalTree).toMatchSnapshot();

Hope this can help on further investigation of this issue.

@cardamo
Copy link

@cardamo cardamo commented Feb 27, 2018

Speaking of workarounds.
I've just added this hack to my jest snapshot tests.

ReactDOM.createPortal = node => node
@reyronald
Copy link

@reyronald reyronald commented Mar 7, 2018

Something else that works is 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.

@kebot
Copy link

@kebot kebot commented Apr 10, 2018

Here is an example of how I did, you can Mock the createPortal with your own logic.

describe('Popover', () => {
  beforeAll(() => {
    ReactDOM.createPortal = jest.fn((element, node) => {
      return element
    })
  })

  afterEach(() => {
    ReactDOM.createPortal.mockClear()
  })

  it('should render correctly with Node or Function', () => {
    const component = renderer.create(
      <Popover active trigger={<button>BUTTON</button>}>this is a Popover</Popover>
    )

    expect(component.toJSON()).toMatchSnapshot()
  })
})

Then your Snaptests will looks like this:

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Popover should render correctly with Node or Function 1`] = `
<div
  className="popover"
>
  <button
    onClick={[Function]}
  >
    BUTTON
  </button>
  <div
    className="bottom show"
    onClick={[Function]}
    style={
      Object {
        "left": 0,
        "position": "absolute",
        "top": 0,
        "zIndex": 800,
      }
    }
  >
    this is a Popover
  </div>
</div>
`;

Any thoughts?

@bvaughn bvaughn self-assigned this May 23, 2018
@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented May 23, 2018

Currently, two React renderers cannot be used at the same time in the way that is being described. (A exception sort of exists for react-art and react-dom- where we intend to support cross render portals, but we have not yet built this.)

The reason this fails is mentioned above- the container types are different. react-dom expects a DOM element (e.g. HTMLDivElement) which children are appended to using container.appendChild(). react-test-renderer expects a wrapper object with a children array. It adds children by calling container.children.push().

I think this should be possible to support in a better way. See PR #12895.

@gaearon
Copy link
Member

@gaearon gaearon commented Aug 17, 2018

For context, latest thinking on this is: #12895 (comment)

@nathan5x
Copy link

@nathan5x nathan5x commented May 20, 2020

bump +

@blissdev
Copy link

@blissdev blissdev commented May 22, 2020

Still encountering this issue.

@RyanPWalker
Copy link

@RyanPWalker RyanPWalker commented May 29, 2020

For those of you who get the error Cannot read property 'tag' of null when mocking the createPortal, assign the original createPortal implementation to a variable, mock the implementation and run your test, then reassign it to the original implementation after the test. Only downside is you would have to do this for every test where you need it.

    it('Renders correctly', () => {
        const oldPortal = ReactDOM.createPortal;
        ReactDOM.createPortal = node => node; // for components that use Portal
        const component = renderer.create(<Portal>{children}</Portal>);
        const tree = component.toJSON();
        
        expect(tree).toMatchSnapshot();

        ReactDOM.createPortal = oldPortal;
    });
@faccudiaz
Copy link

@faccudiaz faccudiaz commented Jun 18, 2020

bump

@tqwewe
Copy link

@tqwewe tqwewe commented Jun 29, 2020

I'm almost there.. just receiving the following error:

TypeError: Cannot set property 'scrollTop' of null

Anyone came across this?

@stale
Copy link

@stale stale bot commented Oct 4, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@ljharb
Copy link
Contributor

@ljharb ljharb commented Oct 4, 2020

bump

@stale stale bot removed the Resolution: Stale label Oct 4, 2020
@KorySchneider
Copy link

@KorySchneider KorySchneider commented Oct 27, 2020

bump, none of the above solutions worked

richardxia added a commit to ShelterTechSF/sheltertech.org that referenced this issue Nov 16, 2020
richardxia added a commit to ShelterTechSF/sheltertech.org that referenced this issue Nov 16, 2020
richardxia added a commit to ShelterTechSF/sheltertech.org that referenced this issue Nov 16, 2020
richardxia added a commit to ShelterTechSF/sheltertech.org that referenced this issue Nov 17, 2020
@gandhirahul
Copy link

@gandhirahul gandhirahul commented Jan 22, 2021

bump

@exaucae
Copy link

@exaucae exaucae commented Apr 24, 2021

@kebot @cardamo, getting type error with your solutions in typescript: type ReactNode is not assignable to type ReactPortal.

@cardamo
Copy link

@cardamo cardamo commented Apr 24, 2021

@chrys-exaucet both of them are in vanilla js, not TypeScript. Since it's a workaround anyway, just make TS to ignore these types like this:

// @ts-ignore
ReactDOM.createPortal = node => node;

or like this

ReactDOM.createPortal = node => node as ReactPortal;
@cardamo
Copy link

@cardamo cardamo commented Apr 24, 2021

I've just tried to reproduce the issue and all seems to work fine with React 17, specifically:

    "@testing-library/jest-dom": "^5.11.9",
    "@testing-library/react": "^11.2.5",
    "react": "^17.0.1",

Was it fixed in 17 or earlier?

@ahummel25
Copy link

@ahummel25 ahummel25 commented Apr 28, 2021

I still have the error mentioned in the below issue. Is there a fix for this when using Material UI? Seems MUI is dependent on this.

ForwardRef(Fade)(...): Nothing was returned from render. This usually means a return statement is missing. Or, to render nothing, return null.

mui-org/material-ui#12237

@exaucae
Copy link

@exaucae exaucae commented May 3, 2021

@cardamo, I experience issues with your specified versions. Here's the related sandbox.

@alpgokcek
Copy link

@alpgokcek alpgokcek commented May 18, 2021

bump

1 similar comment
@misterlocations
Copy link

@misterlocations misterlocations commented Jun 15, 2021

bump

@exaucae
Copy link

@exaucae exaucae commented Jul 9, 2021

Bump!

@bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Oct 1, 2021

Folks: Please stop bumping this issue, or we will lock it. The core team is aware of it.

I will tag it as "React Core Team" so no bot attempts to close it. In return, please trust us to prioritize it appropriately.

@kadikbecerrasoliz
Copy link

@kadikbecerrasoliz kadikbecerrasoliz commented Nov 19, 2021

Here is an example of how I did, you can Mock the createPortal with your own logic.

describe('Popover', () => {
  beforeAll(() => {
    ReactDOM.createPortal = jest.fn((element, node) => {
      return element
    })
  })

  afterEach(() => {
    ReactDOM.createPortal.mockClear()
  })

  it('should render correctly with Node or Function', () => {
    const component = renderer.create(
      <Popover active trigger={<button>BUTTON</button>}>this is a Popover</Popover>
    )

    expect(component.toJSON()).toMatchSnapshot()
  })
})

Then your Snaptests will looks like this:

// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Popover should render correctly with Node or Function 1`] = `
<div
  className="popover"
>
  <button
    onClick={[Function]}
  >
    BUTTON
  </button>
  <div
    className="bottom show"
    onClick={[Function]}
    style={
      Object {
        "left": 0,
        "position": "absolute",
        "top": 0,
        "zIndex": 800,
      }
    }
  >
    this is a Popover
  </div>
</div>
`;

Any thoughts?

Thank you. I can say for certain that mocking the portal function works on a pure javascript environment.

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