wrapper is not updated when using conditional rendering in v3

[yesmeck]

I found that when using conditional rendering, after setProps, wrapper is not updated properly.

// src/Hello.js
import React from 'react';

export default ({ show }) => (
  <div>
    {show ? <span>hello</span> : null}
  </div>
);

// tests/Hello.test.js
import { mount } from 'enzyme';
import React from 'react';
import Hello from '../src/Hello';

test('Hello', () => {
  const wrapper = mount(<Hello show />)

  expect(wrapper.find('span').length).toBe(1)

  wrapper.setProps({ show: false });
  expect(wrapper.find('span').length).toBe(0)

  wrapper.setProps({ show: true });
  wrapper.update(); // even though calling wrapper.update explicitly
  console.log(wrapper.debug()); // no span
  console.log(wrapper.html()); // has span
  expect(wrapper.find('span').length).toBe(1) // failed
});

Here is a reproduction repo https://github.com/yesmeck/enzyme-set-props-bug

[ljharb]

What version of React are you using?

[yesmeck]

"react": "^16.0.0",
"react-dom": "^16.0.0"

Here is the full dependencies https://github.com/yesmeck/enzyme-set-props-bug/blob/master/package.json

[tricoder42]

I can confirm this behavior with [email protected] and enzyme-adapter-react-16. Upgrading to [email protected], but keeping react@15 works fine.

[gregberge]

Same issue for me, update is not working for React 16.

[gregberge]

I investigated it and found the source of the bug:

https://github.com/airbnb/enzyme/blob/master/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js#L66

At the first render, vnode.memoizedProps is the source of truth.
At the second, vnode.alternate.memoizedProps is the source of truth.
At the third, vnode.memoizedProps is the source of truth.

That was it is named alternate I think. We should confirm this from the React team and ask them the correct way to handle it.

Should we alternate the node at each render? @gaearon @sebmarkbage @acdlite

[robin-drexler]

Having the same issue with react adapter ^15.5.0.

Edit:
My issue seems to be slightly different. Will investigate further and open a separate issue if applicable.

[Axbon]

Can confirm, having the same issue with React 16 and Enzyme 3, .html() yields correct result but using .debug() or trying to .find('selector') will yield no result after conditional rendering.

[sebmarkbage]

It's not possible to get the "current" subtree from any particular node.

Meaning that this is not possible to implement this as a shortcut:
https://github.com/airbnb/enzyme/blob/8cf9661a869fcbb93562e8791d6d07bd2ab6b68f/packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js#L182

The only way to get the current tree is to find the root and scan the tree, walking from the root. You shouldn't ever need to touch the .alternate because that will likely lead to the wrong result.

(The reason for this is because we don't actually mutate every node in the commit phase. We only swap the root pointer. That's why that's the only pointer that gives you the source of truth.)

[ajur58]

Same issue with conditional rendering based on state. Using [email protected] and enzyme-adapter-react-16

[gaearon]

Hmm. Ideally Enzyme shouldn't use any fields on _reactInternalFiber. That's why it's internal 😛 My impression was that the 3.0 rewrite addressed this, and everything Enzyme needed is now exposed by React in a safe way. But I probably misunderstood something.

[gregberge]

The React 16 adapter needs to be partially rewritten. I don’t know how to read all tree from instance but I could take a look.

[lelandrichardson]

@sebmarkbage thanks for the context, that's really helpful! i will work on rewriting this. hopefully we can get some cleaner code doing as you suggested.

@gaearon I totally want us to get to this place. The 3.0 rewrite definitely put us in a place where this will be easier to do than before, but we still aren't quite there. I don't think you misunderstood something, I think that when I originally proposed this new architecture, I thought that we would have all of the information needed to upgrade enzyme without using any internals.... I was wrong.

My goal is to eventually create a new top-level API for enzyme (tentatively called deep) which will use react-test-renderer's toTree() method. This, along with the mocking abilities it has, I'm hopeful we will be able to reproduce shallow as a subset of the deep API (a bit of an oxymoron going on here).

That leaves mount, which we won't be able to use react-test-renderer to reproduce, and as a result I think we will continue to need to access internal properties of react unless we can get a toTree() method or something similar that react itself exports. This was the main thing that I didn't previously understand.

I'm hopeful we will be able to slowly move to less and less of a dependence on internals

[gaearon]

That makes sense, thank you for explaining!

[aweary]

For mount, we might be able to adapt ReactTestUtils to support toTree and the other methods we need for mounting and updating props/state. I proposed something along these lines here facebook/react#9505

[marudor]

Seems like I have the same Problem with react adapter ^15.5.0.
(Maybe it is the same @robin-drexler has and I can't seem to find the difference to this issue)

[HillLiu]

Not sure, is it same issue? while I use setState to update render function, I need call shallow.update() to refresh that shallow.html() to get correct content.

Some my unit test
react-atomic/reshow@09a3b6c

[ljharb]

@HillLiu that's an intended change in v3.

[gziolo]

I did some heave debugging for WordPress/gutenberg#2813 and it looks like we are having the same kind of issues that are reported in this thread.

I tried to add recommended wrapper.update(); call after the click event is simulated, but it didn't help. It's quite interesting scenario because the part of the component that depends on the component's state (https://github.com/WordPress/gutenberg/blob/ad316ab1f8cfe60aeeb45ff27d45336bfb214c3c/editor/inserter/menu.js#L384) gets properly updated, but the other part (https://github.com/WordPress/gutenberg/blob/ad316ab1f8cfe60aeeb45ff27d45336bfb214c3c/editor/inserter/menu.js#L366) which also is conditionally rendered based on the components' state doesn't show up in the output of wrapper.debug() call. When I debugged it turned out that React component does everything properly behind the scenes. It looks like Enzyme gets out of sync.

[gaearon]

Btw here is how we find current fiber in React itself. But you really shouldn't depend on this. If there's something we need to expose, just let us know, and we will on our side.

[gregberge]

Thanks @gaearon, I made a fix #1169. I think exposing an API toTree as suggested by @aweary in facebook/react#9505 would be a good idea!

[KittyGiraudel]

@neoziro Thank you for taking the time to fix this. Do you have any idea when this change will be published on npm so we can upgrade? :)

[gregberge]

@hugogiraudel I think they will publish today or tomorrow

[eddiemonge]

Should this be working now? I am using [email protected] but I still get

TypeError: Cannot read property 'rendered' of null

      at Object.getNode (node_modules/enzyme-adapter-react-16/build/ReactSixteenAdapter.js:238:69)
      at new ReactWrapper (node_modules/enzyme/build/ReactWrapper.js:100:33)
      at mount (node_modules/enzyme/build/mount.js:19:10)

[ljharb]

@eddiemonge if you are on the latest enzyme and enzyme-adapter-react-16, please do file a new issue.

[waqasiftikhar2]

I am facing the same issue. Has this been solved in all versions or the fix is version specific?

[ljharb]

@waqasiftikhar2 please file a new issue and we can figure that out.

[hedin-hiervard]

I can confirm this is still an issue.
"enzyme": "3.3.0", "enzyme-adapter-react-16": "1.1.1",

[hamza-hajji]

I'm having this issue as well, wrapper.html() returns the correct html but .find fails

[bhongy]

It works fine for me now with [email protected] and [email protected].

It was shouldComponentUpdate for me. There were a couple tests in my codebase that didn't update with .setProps() even with calling wrapper.update() but those were components that implemented sCU incorrectly—fixing the source code fixed it.

[sidferreira]

@Hedinhiervard using wrapper.update() seem to fix it

[Alebron23]

@Hedinhiervard Same for me wrapper.update() fixed it for me using [email protected], enzyme-adapter-react-16@^1.1.1, and react@^16.2.0. But wrapper.instance().forceUpdate() did not fix it for me. I was also shallow rendering my component.

[developer239]

Make sure that you have enzyme-adapter-react-16 🙂

[SelaO]

Having this issue too, conditional rendering doesn't happen on a component I mounted.

Both wrapper.update() and wrapper.instance().forceUpdate() don't help either.

"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
"enzyme-to-json": "3.3.4",
"react": "16.4.1",

[gilneto8]

Any update regarding this issue? I'm having exactly the same problem as @SelaO mentioned: mounted component with conditional rendering based on state changes. Changing the state inside the test updates the state, but not what is rendered, even after invoking wrapper.update() and wrapper.instance().forceUpdate().

    "enzyme": "^3.7.0",
    "enzyme-adapter-react-16": "^1.6.0",
    "react": "^16.6.1",

[ljharb]

Someone who's still having an issue: please file a new issue, fill out the issue template in full, and link it back to here. Once that issue exists, anyone else should please provide any information that's different from the full issue template post from the new OP, and we'll get this solved.