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

`useEffect` not called when the component is `shallow` renderered #2086

Open
jlandic opened this issue Apr 8, 2019 · 41 comments
Open

`useEffect` not called when the component is `shallow` renderered #2086

jlandic opened this issue Apr 8, 2019 · 41 comments
Projects

Comments

@jlandic
Copy link

@jlandic jlandic commented Apr 8, 2019

Current behavior

useEffect fonction does not seem to be executed when the component is rendered with shallow.

Given the following simple component using useEffect hook to call a method given as a prop:

import React, { useEffect } from 'react';

export const MyComponent = ({ callback }) => {
  useEffect(() => {
    callback('Say something');
  });

  return <div>Hello world!</div>;
};

And the following tests checking that the callback function is indeed called accordingly:

import React from 'react';
import { shallow, mount } from 'enzyme';
import { MyComponent } from '../MyComponent';

describe('MyComponent', () => {
  describe('shallow render', () => {
    it('should call callback when shallow rendered', () => {
      const callback = jest.fn();
      shallow(<MyComponent callback={callback} />);
      expect(callback).toHaveBeenCalled();
    });
  });

  describe('mount', () => {
    it('should call callback when mounted', () => {
      const callback = jest.fn();
      mount(<MyComponent callback={callback} />);
      expect(callback).toHaveBeenCalled();
    });
  });
});

We observe that the method is called as expected when "mounted", but not when using a shallow renderer:

 FAIL  src/tests/MyComponent.test.js
  MyComponent
    shallow render
      ✕ should call callback when shallow rendered (12ms)
    mount
      ✓ should call callback when mounted (24ms)

  ● MyComponent › shallow render › should call callback when shallow rendered

    expect(jest.fn()).toHaveBeenCalled()

    Expected mock function to have been called, but it was not called.

       8 |       const callback = jest.fn();
       9 |       shallow(<MyComponent callback={callback} />);
    > 10 |       expect(callback).toHaveBeenCalled();
         |                        ^
      11 |     });
      12 |   });
      13 |

      at Object.toHaveBeenCalled (src/tests/MyComponent.test.js:10:24)

You may find this reproducible example here.

Expected behavior

When using shallow to render the components, the function passed to useEffect should be executed, as it is when using mount.

Your environment

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.9.0
react 16.8.6
react-dom 16.8.6
react-test-renderer 16.8.6
adapter (below)

Adapter

  • enzyme-adapter-react-16
  • enzyme-adapter-react-16.3
  • enzyme-adapter-react-16.2
  • enzyme-adapter-react-16.1
  • enzyme-adapter-react-15
  • enzyme-adapter-react-15.4
  • enzyme-adapter-react-14
  • enzyme-adapter-react-13
  • enzyme-adapter-react-helper
  • others ( )
@vinicius33

This comment has been minimized.

Copy link

@vinicius33 vinicius33 commented Apr 8, 2019

Same here. Do you have any idea when this is going to be supported? 🙏
Thanks,

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Apr 13, 2019

Hopefully in the next version of enzyme.

@ljharb ljharb added this to v16.8+: Hooks in React 16 Apr 13, 2019
@jnelken jnelken mentioned this issue Apr 30, 2019
2 of 13 tasks complete
@bdwain

This comment has been minimized.

Copy link
Contributor

@bdwain bdwain commented Apr 30, 2019

Was there a plan to support that? I made an issue in react to support this from the shallow renderer with an option. I figured that would be the only way to do it without trying to mock out useEffect or something

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented May 1, 2019

ah, fair enough - you're right that it still won't work in shallow, and that that issue, or mocking it out in enzyme, would be the only way to make it work.

@mangel0111

This comment has been minimized.

Copy link

@mangel0111 mangel0111 commented May 2, 2019

Is there a plan or PR to fix this? Makes hard include hoos/useEffect if we can't test it

@bdwain

This comment has been minimized.

Copy link
Contributor

@bdwain bdwain commented May 2, 2019

@mangel0111 see the previous 2 comments. we're waiting to see if we can add a feature to react which will make it easier to fix this

@tomasro27

This comment has been minimized.

Copy link

@tomasro27 tomasro27 commented May 21, 2019

Having the same issue here. Is there any alternative to creating a snapshot that runs the useeffect hooks?

@MM263

This comment was marked as outdated.

Copy link

@MM263 MM263 commented Jun 5, 2019

setTimeout(() => {
  expect(yourFunction).toHaveBeenCalledTimes(1);
});

This appears to work

Edit: This doesn't work at all, just skips over the test. Waiting for a solution to this now too.

@gonzofish

This comment was marked as outdated.

Copy link

@gonzofish gonzofish commented Jun 6, 2019

@MM263 make sure that the setTimeout callback is even firing. Your test is probably passing because it isn't reaching the assertion in time

@MM263

This comment was marked as outdated.

Copy link

@MM263 MM263 commented Jun 6, 2019

@gonzofish We've just discovered this, you are totally right! Thank you!

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 21, 2019

It looks like the render and update calls just need to be called inside an act() callback and this will flush the effects, whether using shallow rendering or not. I have this working in a hacked together enzyme now. EDIT: I made a mistake, implemented the feature below though.

@bdwain

This comment has been minimized.

Copy link
Contributor

@bdwain bdwain commented Jul 21, 2019

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 21, 2019

@bdwain Sorry I had a bunch of changes to ReactShallowRenderer which I was supposed to remove before adding the act() wrapper. Actually the act() wrapper changed nothing. It's not that hard to make the change but react maintainers don't seem to be interested at the moment.

@bdwain

This comment has been minimized.

Copy link
Contributor

@bdwain bdwain commented Jul 21, 2019

yea i wonder if they would be more interested if it was completed already. If you got it all working maybe post that in a PR and see what they say?

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 21, 2019

@bdwain My PR is here: facebook/react#16168 would be really grateful for your feedback.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 25, 2019

BTW it's possible to get this working from enzyme without having to touch react, but you have to override _createDispatcher in the shallow renderer to call the original version and patch useEffect and useLayoutEffect with my versions from the PR above and return the patched dispatcher. Then after enzyme calls render you just add in my effect calling code from the PR above.

Would be nicer if the code was in React's shallow renderer though.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 27, 2019

I needed this feature like... 5 months ago. Which is about the length of time the react maintainers have been ignoring the issue for. If they don't respond to my PR in another few weeks could I maybe do the private method override thing? I would make enzyme's code test for the existence of the private method first so as not to break if the internals change. Then when (or more like if) react accept the PR or code then it could just be removed. Although it is super hacky :( In my own project I switched from enzyme to the test renderer and do the hack now so I can at least test my effects, but I'd much rather be using enzyme.

@ljharb

This comment has been minimized.

Copy link
Collaborator

@ljharb ljharb commented Aug 21, 2019

@ohjames as long as your tests here are good, that is totally fine.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Aug 21, 2019

@ljharb Cool, I've written the code now. Having trouble testing it. After running the steps listed in CONTRIBUTORS.md I get the following after npm run react 16:

james@lapchil enzyme % npm run react 16                                                                         
                                                                                                                
> enzyme@0.0.1 react /home/james/projects/enzyme                                                                
> sh install-relevant-react.sh "16"                                                                             
                                                                                                                
installing React 16                                                                                             
                                                                                                                
> enzyme@0.0.1 env /home/james/projects/enzyme                                                                  
> babel-node ./env.js "16"                                                                                      
                                                                                                                
rimraf ./node_modules/react                                                                                     
rimraf ./node_modules/react-dom                                                                                 
rimraf ./node_modules/react-addons-test-utils                                                                   
rimraf ./node_modules/react-test-renderer                                                                       
rimraf ./node_modules/create-react-class                                                                        
rimraf node_modules/.bin/npm 
rimraf node_modules/.bin/npm.cmd
npm i
npm WARN checkPermissions Missing write access to /usr/lib/node_modules/npm/node_modules

Hmmm... not sure why it wants to install things to system owned directories. Don't really want to contaminate my system :(

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Aug 21, 2019

Okay... so I decided to run it in a container (I chose node:11) because I didn't want to run things as root just to get this working (and have lost all interest in trying lerna which I thought previously might be a good idea). But get 11 failing tests with a totally clean codebase. Hmm... enzyme's build system is intense and scary.

Here's some examples of failing tests:

  1) Adapter provides node displayNames supports Profiler:                                                      
     AssertionError: expected null to equal 'Profiler'                                                          
      at Context.<anonymous> (packages/enzyme-test-suite/test/Adapter-spec.jsx:1048:47)                         
      at processImmediate (internal/timers.js:443:21)

  2) Adapter provides node displayNames supports ConcurrentMode:                                                
     AssertionError: expected null to equal 'ConcurrentMode'                                                    
      at Context.<anonymous> (packages/enzyme-test-suite/test/Adapter-spec.jsx:1052:53)                         
      at processImmediate (internal/timers.js:443:21)

  10) (uses jsdom) mount Profiler wrapped: with console throws: mounts without complaint:                       
     AssertionError: expected [Function] to not throw an error but 'EvalError: Warning: React.createElement: typ
e is invalid -- expected a string (for built-in components) or a class/function (for composite components) but g
ot: %s.%s%s' was thrown                                                                                         
      at Context.<anonymous> (packages/enzyme-test-suite/test/ReactWrapper-spec.jsx:865:9)                      
      at processImmediate (internal/timers.js:443:21)                                                           
                                                                                                                
  11) (uses jsdom) mount Profiler wrapped: with console throws: "after each" hook: afterEachWithOverrides:
     Uncaught EvalError: The above error occurred in the <SomeComponent> component:                             
    in SomeComponent (created by WrapperComponent)                                                              
    in WrapperComponent

Now I kinda feel like giving up.

@ljharb

This comment was marked as off-topic.

Copy link
Collaborator

@ljharb ljharb commented Aug 21, 2019

It doesn’t need root unless npm install -g needs root, which it doesn’t if you’re using nvm.

@insidewhy

This comment was marked as off-topic.

Copy link

@insidewhy insidewhy commented Aug 21, 2019

@ljharb Most of the world seems to have moved onto docker-compose instead of nvm now, so I don't think mandating nvm is a good idea. IMO using docker-compose is a way better approach for most people/projects as it fixes a tonne of other system difference problems as well.

@LeonardPurschke

This comment was marked as spam.

Copy link

@LeonardPurschke LeonardPurschke commented Aug 30, 2019

Hi,
I am also encountering this issue.
I really appreciate your work and would like to have that feature, too :(

@insidewhy

This comment was marked as off-topic.

Copy link

@insidewhy insidewhy commented Aug 30, 2019

@LeonardPurschke Don't worry I've completed the work and it should be available soon. I'm just waiting on the enzyme Devs to fix the reliability of enzyme's tests so I can open the PR.

@ljharb

This comment was marked as off-topic.

Copy link
Collaborator

@ljharb ljharb commented Aug 31, 2019

@ohjames that's a very bold, highly untrue (travis-ci uses nvm, for example, and the number of travis jobs ran dwarfs almost every other usage), and unhelpful claim. Since I'm the primary maintainer, I'm going to mandate whatever makes my life easier, and docker isn't it.

@insidewhy

This comment was marked as off-topic.

Copy link

@insidewhy insidewhy commented Aug 31, 2019

Okay, if you care more about your own development experience on a community project than that's your prerogative, but it doesn't change the fact that docker is a far better system for environmental isolation than nvm. You chose Travis as your example, but Travis also supports docker, and CircleCI, which is the CI of react and millions others, is built on top of docker. It seems evident from the unreliability of the tests when run in containers, even on versions of react other than 16.9, that enzyme is highly coupled to your personal environment and the one on Travis. If they were run via containers this issue would be moot.

@monkey-butler

This comment has been minimized.

Copy link

@monkey-butler monkey-butler commented Sep 3, 2019

Running into this issue when I try to use shallow or mount in enzyme tests:

Invariant Violation: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://fb.me/react-invalid-hook-call for tips about how to debug and fix this problem.

  65 | export function ProgrammeCard(props):React.ReactElement{
  66 | 
> 67 |   const classes = useStyles({});
  68 | 
  69 |   let Pinned =  false;
  70 |   if (props.item[definitions.pinnedColumn + 'Id'] !== null){

  at invariant (node_modules/react/cjs/react.development.js:88:15)
  at resolveDispatcher (node_modules/react/cjs/react.development.js:1436:28)
  at Object.useContext (node_modules/react/cjs/react.development.js:1441:20)
  at useTheme (node_modules/@material-ui/styles/useTheme/useTheme.js:15:25)
  at node_modules/@material-ui/styles/makeStyles/makeStyles.js:244:39
  at Object.ProgrammeCard (src/webparts/wfd1/components/ProgrammeCard.tsx:67:33)
  at Suite.Object.<anonymous>.describe (src/webparts/wfd1/test/ProgrammeCard.test.ts:25:5)
  at Object.<anonymous> (src/webparts/wfd1/test/ProgrammeCard.test.ts:23:1)

The code I want to test is a small component that renders a material-ui card in an spfx web part, my setup is as follows:

React v16.8.5
React-DOM v16.8.5
enzyme v3.10.0

The test suite is

> //Tests for ProgrammeCard
> 
> /// <reference types="jest" />
> 
> import * as React from 'react';
> import { configure, mount, shallow } from 'enzyme';
> import * as Adapter from 'enzyme-adapter-react-16';
> import * as testdata from '../testdata/testdata';
> 
> 
> configure({ adapter: new Adapter() });
> function updateItem(){
>   console.log('Pass');
> } 
> import {ProgrammeCard} from '../components/ProgrammeCard';
> const Title = testdata.Programmes[0].ProgrammeName;
> const validProps = {
>   currentUser:testdata.UserProfile,
>   item:testdata.Programmes[0],
>   updateItem:updateItem(),
>   variant:'my'
>    };
> describe ('Check 3 variants of ProgrammeCard', () => {
>   const myProgramme = mount(
>     ProgrammeCard(validProps)
>   );
>   expect(myProgramme.text()).toEqual('I am Groot');
> });
> 
> 

Has anyone got this working?

@dikamilio

This comment has been minimized.

Copy link

@dikamilio dikamilio commented Oct 18, 2019

I've found sollution that you need to add import in your test file:
import React, {useEffect} from 'react';

And then in beforeEach function add following line:
useEffect = jest.spyOn(React, "useEffect").mockImplementation(f => f());

Also in your Component use React.useEffect instead of just useEffect
This will trigger useEffect function using shallow

@windmaomao

This comment has been minimized.

Copy link

@windmaomao windmaomao commented Oct 27, 2019

@dikamilio but this wouldn't allow to test any behavior after any monitored value changed, ex.

  useEffect(() => {}, [params])
@falvariza

This comment has been minimized.

Copy link

@falvariza falvariza commented Nov 4, 2019

I made it work with the react-dom/test-utils act helper.

const myCB = jest.fn();
act(() => {
  mount(<MyComponent myCB={myCB} />);
});
expect(myCB).toHaveBeenCalled();
@cecedille1

This comment has been minimized.

Copy link

@cecedille1 cecedille1 commented Nov 5, 2019

I have made my own version that will run the effect if the dependences changes

import React from "react";

/** Workarround for useEffect. It will run the effect synchronously if any deps changes */
function almostUseEffect() {
	return jest.spyOn(React, 'useEffect').mockImplementation((cb, deps) => {
		// create a new symbol, different at each run and save the first one
		const firstRun = Symbol();
		const isFirstRun = React.useMemo(() => firstRun, []) === firstRun;
		const ref = React.useMemo(() => ({
			current: deps,
		}), []);
		const last = ref.current;

		// compare the last known version of deps with the current one
		const changed = deps && last.some((value, i) => value !== deps[i]);

		if (isFirstRun || changed) {
			ref.current = deps;
			// run the callback if it changed
			cb();
		}
	});
};
@matthewharwood

This comment has been minimized.

Copy link

@matthewharwood matthewharwood commented Nov 21, 2019

Been open since Apr 8th... is this being worked on?

@NathanCH

This comment has been minimized.

Copy link

@NathanCH NathanCH commented Dec 3, 2019

FWIW, this issue now appears first on Google when searching "enzyme test useEffect".

If you're coming from Google, and because it wasn't mentioned above, useEffect is not supported when using shallow.

@adolfoportilla

This comment has been minimized.

Copy link

@adolfoportilla adolfoportilla commented Dec 3, 2019

I posted a stackoverflow question to see if anybody answers it.

@sweetleon

This comment has been minimized.

Copy link

@sweetleon sweetleon commented Dec 4, 2019

Here's a solution from a colleague of mine at CarbonFive:
https://blog.carbonfive.com/2019/08/05/shallow-testing-hooks-with-enzyme/
TL;DR: jest.spyOn(React, 'useEffect').mockImplementation(f => f())

@stvpalumbo

This comment has been minimized.

Copy link

@stvpalumbo stvpalumbo commented Dec 4, 2019

@sweetleon what you suggest is more of a work around, and not a particularly robust one if you need to test that useEffect fires when a value changes (or if the component unmounts).

I've had success with getting realistic behavior by using a full mount via enzyme's mount capability. This will cause a full tree render, BUT only if you let it. It is extra work but to avoid the full tree render, I mock out component constructors via jest.mock capability.

example:

// Component.jsx
import React from 'react';
export default () => {
  return <div>component with lots of nested other components we don't want to test</div>;
}
// App.jsx
import React, { useEffect, useState } from 'react';
import Component from './Component.jsx';

export default (props) => {
  const [capital, setCapital] = useState('');
  useEffect(() => {
    setCapital(props.importantProp.toUpperCase());
  }, [props.importantProp]);
  return (
    <div>
       <div className="capital-value">{capital}</div>
       <Component />
    </div>
  );
}
// App.spec.jsx
import { mount } from 'enzyme';
import App from './App.jsx';
jest.mock('./Component.jsx', () => {
  return () => null;
});
it('should do the thing', () => {
  const wrapper = mount(<App importantProp="some value" />);
  expect(wrapper.find('.capital-value').text()).toBe('SOME VALUE');
  wrapper.setPros({
    importantProp: 'new value',
  });
  wrapper.update(); // this is needed for some reason
  expect(wrapper.find('.capital-value').text()).toBe('NEW VALUE');
});

I've adopted this approach in my code base until shallow works as expected. I hope this helps...

@itsgracian

This comment has been minimized.

Copy link

@itsgracian itsgracian commented Dec 5, 2019

I made it work with the react-dom/test-utils act helper.

const myCB = jest.fn();
act(() => {
  mount(<MyComponent myCB={myCB} />);
});
expect(myCB).toHaveBeenCalled();

I have tried this, still not working.

@shiraze

This comment has been minimized.

Copy link

@shiraze shiraze commented Dec 12, 2019

Please stop posting solutions/issues that relate to mount(). This issue relates to limitations of shallow() when trying to confirm expected changes made by useEffect when using setProps or update().
There are already ample solutions that show how mount() can be used.
This is also not the place to discuss the pros and cons of shallow/deep rendering.
Thanks!

@inkless

This comment has been minimized.

Copy link

@inkless inkless commented Jan 28, 2020

My workaround for now, so you don't have to use React.useEffect, you can still use import React, { useEffect } from 'react'; in your codebase.

jest.mock('react', () => ({
  ...jest.requireActual('react'),
  useEffect: (f) => f(),
}));
@Aljal

This comment has been minimized.

Copy link

@Aljal Aljal commented Feb 4, 2020

Shallow still does not trigger useEffect and this issue is still open... can we expect it to work at some point ?

Ps: #1938 (comment) seems to answer my question but this issue is still open so is it an issue or not?

@thedanchez

This comment has been minimized.

Copy link

@thedanchez thedanchez commented Feb 5, 2020

@Aljal There is another active GitHub thread, facebook/react#17321 ,where there is discussion around the future of the Shallow Renderer with regards to eventually supporting the behavior being discussed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
React 16
  
v16.8+: Hooks
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.