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

Add option in shallow renderer to run effects/componentDidUpdate/componentDidMount #15275

Open
bdwain opened this issue Mar 31, 2019 · 23 comments
Open

Comments

@bdwain
Copy link

@bdwain bdwain commented Mar 31, 2019

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

What is the current behavior?
the shallow renderer does not run componentDidUpdate, componentDidMount, or useEffect functions. (I'll call them effect functions for short)

What is the expected behavior?
See this enzyme issue for more details about where this request is coming from. But the general idea is that it is often nice to run effect functions even when shallow rendering, rather than having to use full rendering on those specific tests.

If the shallow renderer provided an option to run the effect functions, it would allow people who test with shallow rendering to more easily test their components. Enzyme currently supports this in class components by calling componentDidUpdate/mount directly on the component instance, but this would be a much harder thing to do for hooks, since they are usually anonymous.

Enzyme used to not allow this at all, but then added an option to turn on this behavior in their shallow renderer, before finally turning it on by default and then adding an option to turn it off. It didn't seem to cause too many issues for them, so I think this approach could work well for the React shallow renderer as well. Obviously I'm only asking for an option to turn it on now, not to change the default or anything.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
All and no it was never supported AFAIK

@bdwain

This comment has been minimized.

Copy link
Author

@bdwain bdwain commented Apr 30, 2019

Any more thoughts on this? I'd be happy to attempt a PR to add this option if there's is no opposition and it's just a matter of prioritizing it.

@bdwain

This comment has been minimized.

Copy link
Author

@bdwain bdwain commented May 8, 2019

I'll start working on a PR

chenesan added a commit to chenesan/enzyme that referenced this issue May 11, 2019
Once the [shallow renderer can run useEffect](facebook/react#15275) we should reopen these tests.
ljharb added a commit to chenesan/enzyme that referenced this issue May 11, 2019
Also reduce the timeout from 1e3 to 100.

Skip shallow tests until [shallow renderer can run useEffect](facebook/react#15275) is fixed.
@chenesan chenesan mentioned this issue Jun 11, 2019
5 of 7 tasks complete
@agrasley

This comment has been minimized.

Copy link

@agrasley agrasley commented Jun 24, 2019

Any chance we could get some eyes from a maintainer on the pull request for this issue? It's been sitting there for a while and it would be nice to have this support for testing hooks in libraries like enzyme. Thanks!

@hweeks

This comment has been minimized.

Copy link

@hweeks hweeks commented Jun 24, 2019

This feature is vital for us to be able to move over to some of these newer paradigms in React. Is there anything we can do to help bump this ticket for visibility?

@pmoeller91

This comment has been minimized.

Copy link

@pmoeller91 pmoeller91 commented Jun 25, 2019

Likewise this is a big deal. Hooks support on shallow rendering is crucial.

@yanlipnican

This comment has been minimized.

Copy link

@yanlipnican yanlipnican commented Jun 27, 2019

I hope this get done soon. We would like to use hooks.

@nagman

This comment has been minimized.

Copy link

@nagman nagman commented Jul 4, 2019

This feature is crucial. As my whole project only uses hooks, I can test none of useEffect functions with Enzyme.

Could this issue be assigned to someone?

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 21, 2019

@nagman Consider it assigned to me! I've opened a PR with the feature fully implemented. I'm happy to do whatever it takes to get this merged if a maintainer can make time to review it.

insidewhy added a commit to insidewhy/react that referenced this issue Jul 21, 2019
insidewhy added a commit to insidewhy/react that referenced this issue Jul 21, 2019
@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Jul 23, 2019

Might also be worth up-voting my PR here: #16168 might get it more visibility with the react maintainers.

@thedanchez

This comment has been minimized.

Copy link

@thedanchez thedanchez commented Oct 11, 2019

Any updates on when this PR would get assigned and merged in? Really need Hooks fully supported for Enzyme Shallow Rendering.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Oct 12, 2019

@thedanchez Facebook won't support shallow rendering the their shallow renderer anymore so they won't merge my PR or any others relating to that area. Don't think it's pragmatic given the work is already there. They offered to move the shallow renderer into its own package and hand over maintenance, but when we asked a few questions relating to how this would happen, they went back into ignore mode.

@Sonic12040

This comment has been minimized.

Copy link

@Sonic12040 Sonic12040 commented Oct 12, 2019

@ohjames - Can you link me to a source for that discontinued support for shallow rendering?

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Oct 12, 2019

@thedanchez

This comment has been minimized.

Copy link

@thedanchez thedanchez commented Oct 22, 2019

I'm trying to keep that issue thread active by checking in on a weekly basis. I really think this feature needs to be added ASAP.

Don't know what's keeping them from handing ownership of react-shallow-renderer over to Airbnb or to the community at larger sooner rather than later.

@Sonic12040

This comment has been minimized.

Copy link

@Sonic12040 Sonic12040 commented Oct 22, 2019

Thanks for keeping that active! I'm sure given the number of tickets they must receive (internal and external), that it might be some time before they can migrate a section of the code out.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Oct 23, 2019

@thedanchez the shallow renderer uses a bunch of methods in the shared folder that aren't exported by react. We asked them about it but they stopped responding. You're probably better off giving up on the shallow renderer at this point, Facebook don't care to maintain anything that isn't useful to Facebook and are clearly too busy to respond to anything not in their own interests. They say shallow renderer encourages brittle tests. They didn't give justification but they believe it. People in the PR thread gave arguments with good justifications against this but they aren't listening.

@insidewhy

This comment has been minimized.

Copy link

@insidewhy insidewhy commented Oct 23, 2019

@thedanchez BTW enzyme isn't maintained by Airbnb anymore, mostly by a single develop who used to work at Airbnb. I doubt Airbnb would be interested in maintaining the shallow renderer.

@thedanchez

This comment has been minimized.

Copy link

@thedanchez thedanchez commented Oct 23, 2019

@ohjames oh man, I overlooked that detail. As far as shallow rendering based testing goes, what do you guys think is the best course of action for those who are using React Hooks in their components? I'd really like to avoid having to do a full mount in testing...

@tsolman

This comment has been minimized.

Copy link

@tsolman tsolman commented Oct 23, 2019

@kumar303

This comment has been minimized.

Copy link

@kumar303 kumar303 commented Oct 23, 2019

@tsolman This explains how to unsubscribe from issue notifications.

@thedanchez The viable and supported alternative right now is to test with a full mount and use something like jest mocks to prevent nested components from rendering. The react-testing-library FAQ has an example of mimicking shallow rendering with this technique but I dislike how all nested components need to be mocked manually.

If anyone knows of a way to automatically mock all nested components but not the component under test, please let me know. I've been looking into implementing something like that, including support for "unwrapping" HOCs.

@sombreroEnPuntas

This comment has been minimized.

Copy link

@sombreroEnPuntas sombreroEnPuntas commented Dec 10, 2019

While this is solved, here's a workaround:

// index.tsx
import React from 'react';

export const Component: React.FunctionComponent = () => {
  React.useEffect(() => {
    console.log('Rawr!');
  }, []);

  return <>{'🦕'}</>;
};
// index.test.tsx
import React from 'react';
import { shallow } from 'enzyme';

import { Component } from './index';

jest.spyOn(React, 'useEffect').mockImplementation(f => f());
console.log = jest.fn();

describe(`Component`, () => {
  it('should greet on mount', () => {
    shallow(<Component />);
    expect(console.log).toHaveBeenCalledTimes(1);
  });
});
@shiraze

This comment has been minimized.

Copy link

@shiraze shiraze commented Dec 12, 2019

Unfortunately, I think the only way forward is to use mount(), but that doesn't mean you need to do a "full mount" of all subcomponents. Tests can still mock sub-components so that they're rendered with a mock, e.g.

jest.mock("./mySubComponent", () => () => "MySubComponent");

If the above mocked item needs to be referenced in tests, it needs to be imported (and will then get mocked by above), and can then be referenced by with the import name, e.g.

import MySubComponent from "./mySubComponent";
...
...
const findComponent = wrapper.find(MySubComponent);
@michael-ecb

This comment has been minimized.

Copy link

@michael-ecb michael-ecb commented Jan 21, 2020

Hi,
is it an active issue ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.