[Tests] add hooks tests #2008

chenesan · 2019-02-07T09:25:38Z

This PR adds support for [basic hooks API] (https://reactjs.org/docs/hooks-reference.html#basic-hooks) (useState, useEffect, useContext, related issue #1938 #1996). Currently I'm not sure what hooks API is already working in enzyme shallow / mount and what isn't, so I'll start from adding test cases to confirm the current supports. I'll try to finish the following checklist in the next few days. Also tell me if it's better to have single PR for each api :-)

useState
- mount
  - get value from hooks
  - set value from hooks and rendered element should be updated
- shallow
  - get value from hooks
  - set value from hooks and rendered element should be updated (that's a bug in react shallow renderer, fixed in react after react@16.8.5, see [Shallow] Implement setState for Hooks and remount on type change facebook/react#15120)

I've added some test cases for useState and found it doesn't work in shallow to setState.

(Updated: Created a checklist in #2011 to try to track any issue / PR in hooks, and concentrate on support useState in this PR)

wodenx · 2019-02-07T15:43:24Z

Would you consider adding useRef to the shortlist?

chenesan · 2019-02-08T01:36:58Z

@wodenx I just created #2011 to try to track all of the hooks API support. And in this PR I'd like to work on useState. Feel free to open issue / PR for useRef :)

ljharb · 2019-02-08T07:39:48Z

packages/enzyme-test-suite/test/ReactWrapper-spec.jsx

+        expect(wrapper.find('div').length).to.equal(1);
+        expect(wrapper.find('div').text()).to.equal('0');
+      });
+      it('handles setState returned from useState', () => {


Suggested change

it('handles setState returned from useState', () => {

it('handles setState returned from useState', () => {

@ljharb Can we add setprops test case to simulate componentwillReceiveProps and test using act ?

sure, that sounds great.

ljharb · 2019-02-08T07:40:33Z

packages/enzyme-test-suite/test/ReactWrapper-spec.jsx

+    describeIf(is('>= 16.8'), 'hooks', () => {
+      it('handles useState', () => {
+        const ComponentUsingStateHook = () => {
+          const [count] = React.useState(0);


this should be pulled in react-compat, and not accessed off of React.

perrin4869 · 2019-02-08T14:07:57Z

Hm... Any clue how you'll get this working? The upstream shallow renderer doesn't seem to support this

hisapy · 2019-02-08T21:45:04Z

Hi ...

I just upgraded to React v16.8.1 and I started having these warnings when testing a Hooks based component.

console.error ../../node_modules/react-dom/cjs/react-dom.development.js:506
    Warning: An update to _default inside a test was not wrapped in act(...).
    
    When testing, code that causes React state updates should be wrapped into act(...):
    
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */
    
    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://fb.me/react-wrap-tests-with-act

Is this something that should be handled by enzyme? I didn't have these warnings when I was at the Hooks alphas

ljharb · 2019-02-08T21:54:29Z

@hisapy enzyme has zero support for hooks right now; as for the warnings, that's because react added them between the alphas and the real release.

hisapy · 2019-02-08T22:00:04Z

Thx for the quick answer @ljharb ... I wish I could get rid of the warnings ...

Right now I'm trying to use act to see where it takes me.

Just for the record, I'm not actually testing the hooks. My component useState but in my tests I just mount the component and do the usual simulate stuff, like I would without hooks.

chenesan · 2019-02-09T07:32:06Z

@perrin4869 I'd try to see what I can do in react shallow renderer.

Jessidhia · 2019-02-18T03:04:46Z

act() should be used by any React code that may interact with state, whether you use hooks or not (it'll be important for ConcurrentMode too). It's expected to be reexported and/or automatically wrapped by testing libraries. See https://github.com/threepointone/react-act-examples/blob/master/README.md

ljharb · 2019-02-18T06:30:49Z

It seems like something enzyme could/should perhaps abstract away from the user?

chenesan · 2019-02-28T03:50:43Z

I think that the failed shallow test is related to facebook/react#14840 . After facebook/react#14841 fixes the issue the test should pass.
As for act() I guess we could wrap the call inside returned render of ReactSixteenAdapter.createMountRenderer. But I'm not sure if this should be included in this PR.

ljharb · 2019-03-05T23:08:35Z

@chenesan we should probably merge the act change in a separate PR first.

chenesan · 2019-03-06T00:16:41Z

@ljharb I would try to work on act() this week with some use cases in https://github.com/threepointone/react-act-examples/blob/master/README.md

(Update: Send PR in #2034)

pgangwani · 2019-03-06T03:00:39Z

@ljharb @chenesan : I would like to contribute here. Please help me to get kickstarted. Reading Contributing guide.

chenesan · 2019-03-06T14:03:19Z

Hi @pgangwani glad to hear this. What could I do to help you?
If you're not sure where to start maybe you could find issues with help wanted label. Or maybe you could help add some test cases for react hooks

pgangwani · 2019-03-07T02:08:16Z

I want to start with test cases.

…

On Wed 6 Mar, 2019, 8:25 PM Yi-Shan, Chen, ***@***.***> wrote: Hi @pgangwani <https://github.com/pgangwani> glad to hear this. What could I do to help you? If you're not sure where to start maybe you could find issues with help wanted label <https://github.com/airbnb/enzyme/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22>. Or maybe you could help add some test cases for react hooks <#2011> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2008 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANpzLENY29DZcF6jBCQ9l20sFeOoMSlmks5vT9b-gaJpZM4am4xH> .

ljharb · 2019-03-07T07:45:57Z

@pgangwani please post a link to a branch on #2029, and i'll cherry-pick it into that PR.

pgangwani · 2019-03-07T16:50:59Z

@ljharb : Is there any chatroom ? As I am new, may need little help to start contributing so kickstart well and try sinking in the pattern to work with you

chenesan · 2019-03-07T23:54:46Z

Hi @pgangwani there is gitter chatroom :-)

pgangwani · 2019-03-08T17:24:03Z

@chenesan Added PR #2041 for test cases. Please review

chenesan · 2019-03-09T14:36:14Z

@pgangwani Glad to see your work! I'm not a maintainer here, though. So I have no permission to approve the changes and I think my review doesn't count a lot. Let's wait for @ljharb 's review.

pgangwani · 2019-03-10T19:30:21Z

@chenesan @ljharb All these cases are already covered in #2041

chenesan · 2019-03-11T02:27:59Z

@pgangwani good to see #2041 ;)
@ljharb If these cases are included in #2041, should I close this?

chenesan · 2019-05-29T06:08:34Z

@Acesmndr I believe that works with shallow after react@16.8.5 (See facebook/react#15120)

Acesmndr · 2019-05-31T07:16:23Z

@chenesan I tried but even with React v16.8.6 Enzyme V3.9.0 and Enzyme Adapter v1.13.2 The test is failing. Here is a sample file along with its specs.

import React, { useLayoutEffect, useState } from 'react';

type FormElement = React.FocusEvent<HTMLInputElement>;

const NewForm: React.FC = () => {
  const [email, setEmail] = useState<string>('');
  useLayoutEffect(() => {
    console.log('it should be called but isn\'t called', email);
  }, [email]);

  const handleEmailChange = (evt: FormElement) => {
    const emailValue = (evt.target as HTMLInputElement).value.trim();
    setEmail(emailValue);
    console.log('This gets called but the setEmail is not called');
  };
  
  return (
    <form>
      <input
        id="batman"
        value={email}
        onChange={handleEmailChange}
      />
    </form>
  );
};

export default NewForm;

and the specs:

import * as React from 'react';
import { shallow } from 'enzyme';

import NewForm from './test';

describe('Form', () => {
  let container;

  beforeAll(() => {
    container = shallow(<NewForm />);
  });
  describe('email', () => {
    it('sets value', () => {
      container.find('input').prop('onChange')({
        target: {
          value: 'acesmndr@gmail.com',
        },
      });
      expect(container.find('input').prop('value')).toEqual('acesmndr@gmail.com');
      // console.log(container.find('input').props());
    });
  });
});

And the error it's throwing is that email isn't updated even though setEmail is called.

Am I doing anything wrong?

yuritoledo · 2019-05-31T11:22:21Z

@Acesmndr
Have u tried like this?

<input
  id="batman"
  value={email}
  onChange={e => setEmail(e.target.value)}
/>

Acesmndr · 2019-06-04T18:19:12Z

@yuritoledo Must be one of the dependencies in my project as it didn't work even with that approach. But once I setup everything from scratch using a boilerplate it did indeed work (even my typescript code). :) Must be some rogue npm package which I need to figure out.

JonRCahill · 2019-06-08T08:06:20Z

I am unable to get shallow working with the useState hook either. Maybe it is because I am using react-native?

My code is as follows:

import React, { useState } from "react";
import { View, Button } from "react-native";
import { shallow } from "enzyme";

const Example = function({ button2Press }) {
const [name, setName] = useState("");

  return (
    <View>
      <Button title="Button 1" onPress={() => setName("Hello")} />
      <Button title="Button 2" onPress={() => button2Press(name)} />
    </View>
  );
};

describe("Example", () => {
  it("updates the state", () => {
    const button2Press = jest.fn();
    const wrapper = shallow(<Example button2Press={button2Press} />)
    const button1 = wrapper.findWhere(node => node.prop("title") === "Button 1")
                        .first();
    const button2 = wrapper.findWhere(node => node.prop("title") === "Button 2")
                        .first();

    button1.props().onPress();
    button2.props().onPress();

    expect(button2Press).toHaveBeenCalledWith("Hello");
  });
});

The error I get is that button2Press is always called with an empty string rather then what is being set in state.

I am using the dependencies:

react 16.8.6
react-native 0.59.9
enzyme 3.9.0
enzyme-adapter-react-16 1.13.1

ljharb · 2019-06-08T20:29:22Z

@BobaFaux update to enzyme v3.10 and try again. Either way tho, this is a PR to add useState support, so it's likely that until this is merged, it won't work.

chenesan · 2019-06-09T01:21:43Z

@BobaFaux Try to call update() after the first button pressed and find the button2 again:

const button1 = wrapper.findWhere(node => node.prop('title') === 'Button 1')
        .first();
      button1.props().onPress();
      wrapper.update();
      const button2 = wrapper.findWhere(node => node.prop('title') === 'Button 2')
        .first();
      button2.props().onPress(); 
      wrapper.update();
      expect(button2Press).toHaveBeenCalledWith("Hello");

JonRCahill · 2019-06-09T07:25:06Z

@chenesan thanks that worked for me. I had tried the wrapper.update(); before but my problem was then finding the element again after update is called.

ljharb

I've rebased this on top of a commit that adds a new way to share hook tests across shallow and mount. cc @chenesan for final tweaks to get this in

ljharb · 2019-06-10T18:43:25Z

packages/enzyme-test-suite/test/shared/hooks/custom.jsx

+      });
+    });
+
+    // todo: enable when useEffect works in the shallow renderer


what's the github issue this is linked to?

facebook/react#15275

packages/enzyme-test-suite/test/shared/hooks/custom.jsx

ljharb · 2019-06-10T18:43:46Z

packages/enzyme-test-suite/test/shared/hooks/useContext.jsx

+        );
+      };
+
+      // TODO; useContext seems to not work in the shallow renderer's initial render


can we file a bug on react about this?

This seems not a bug in react side -- I've tried to write a test to render text with the context initial value and that works in shallow. I think it's more about the bug mentioned in #2008 (comment) . But I don't know where the related issue is (e.g. does enzyme has supported .dive() into inner component to get the value from outside provider in shallow?)

yes, ideally that is supported.

This seems like a bug. It looks like even we wrap a Provider outside UiComponent, the context value read by useContext still does not change

ljharb · 2019-06-10T18:44:20Z

packages/enzyme-test-suite/test/shared/hooks/useContext.jsx

+      });
+    });
+
+    // TODO: useContext: enable when shallow dive supports createContext


and also add the link here

See #2008 (comment)

ljharb · 2019-06-10T18:44:30Z

packages/enzyme-test-suite/test/shared/hooks/useEffect.jsx

 } from '../../_helpers/react-compat';

 export default function describeUseEffect({
  hasHooks,
  Wrap,
  isShallow,
 }) {
-  describeIf(hasHooks, 'hooks: useEffect', () => {
+    // TODO: enable when the shallow renderer fixes its bug


do we have an issue link here?

ljharb · 2019-06-10T18:44:55Z

packages/enzyme-test-suite/test/shared/hooks/useEffect.jsx

+        expect(setDocumentTitle.args).to.deep.equal([[expectedCountString(0)]]);
+      });
+
+      // TODO: useEffect fixme


why is this broken on mount?

Not broken, just need a .act wrap; I'd fix this with .simulate .

ljharb · 2019-06-10T18:45:10Z

packages/enzyme-test-suite/test/shared/hooks/useState.jsx

+        );
+      }
+
+      // TODO: fixme when useEffect works in the shallow renderer


let's link the issue here too

packages/enzyme-test-suite/test/shared/hooks/useState.jsx

chenesan · 2019-06-11T01:24:07Z

I think those todos are not from this PR originally but the rebased code in #2041 ; I'll look into them to make progress of #2011, though.

chenesan · 2019-06-11T03:23:33Z

@ljharb Just responsed all the comments and also fixed the lint issues. could be reviewed again :-)

chenesan · 2019-06-11T06:49:44Z

It's strange the the statement in describe of useContext test is still running under React v16.2 even though we have describeIf(hasHook, ...) check, which cause ci failed. I just moved common logic outside of it() into it().

ljharb · 2019-06-11T06:57:50Z

@chenesan the describe bodies run even if they're skipped, so they still have to work in all versions.

…react-hooks [Hooks Testing] Added Test cases for useState, useEffects & customHooks

rodoabad · 2019-07-30T17:33:29Z

Hey @chenesan were you able to get this to work for shallow.

chenesan mentioned this pull request Feb 8, 2019

React hooks support checklist #2011

Open

14 tasks

chenesan changed the title ~~[WIP] Support React hooks~~ [WIP] Support React hooks useState Feb 8, 2019

ljharb added this to v16.8+: Hooks in React 16 Feb 8, 2019

ljharb requested changes Feb 8, 2019

View reviewed changes

swapkats mentioned this pull request Feb 8, 2019

Test custom async hooks #2012

Open

rodrigopr mentioned this pull request Feb 8, 2019

Don't wrap functional components in react >=16.5 to shallow render #2014

Merged

chenesan force-pushed the react-hooks branch from 9f3439a to dee77bd Compare February 28, 2019 03:35

[Tests] add useState, more useEffect, custom hooks

34a7359

ljharb force-pushed the react-hooks branch from 03318c8 to 9a621f7 Compare June 10, 2019 18:40

ljharb requested changes Jun 10, 2019

View reviewed changes

ljharb added API: mount API: shallow Tests labels Jun 10, 2019

chenesan added 2 commits June 11, 2019 10:26

[Tests] fix custom form input hook test

9abded2

[Tests] useEffect: Fix set document title test

d9fa3f0

chenesan force-pushed the react-hooks branch 2 times, most recently from b458b99 to aae8ad1 Compare June 11, 2019 06:47

ljharb force-pushed the react-hooks branch from aae8ad1 to 5b08c0e Compare June 11, 2019 07:15

ljharb changed the title ~~[WIP] Support React hooks useState~~ [Tests] add hooks tests Jun 11, 2019

ljharb approved these changes Jun 11, 2019

View reviewed changes

ljharb force-pushed the react-hooks branch from 3f37b3f to d9fa3f0 Compare June 11, 2019 18:00

ljharb merged commit d9fa3f0 into enzymejs:master Jun 11, 2019

ljharb added a commit that referenced this pull request Jun 11, 2019

Merge pull request #2041 from pgangwani/master + #2008 from chenesan/…

081554e

…react-hooks [Hooks Testing] Added Test cases for useState, useEffects & customHooks

damianpm mentioned this pull request Jul 30, 2019

Improve Service Discovery Errors (AKA improve New API form) 3scale/porta#1015

Merged

4 tasks

juanpicado mentioned this pull request Aug 25, 2019

chore: update material-ui@4.x verdaccio/ui#123

Merged

8 tasks

	it('handles setState returned from useState', () => {

	it('handles setState returned from useState', () => {

[Tests] add hooks tests #2008

[Tests] add hooks tests #2008

Conversation

chenesan commented Feb 7, 2019 • edited

wodenx commented Feb 7, 2019

chenesan commented Feb 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrin4869 commented Feb 8, 2019

hisapy commented Feb 8, 2019

ljharb commented Feb 8, 2019

hisapy commented Feb 8, 2019

chenesan commented Feb 9, 2019

Jessidhia commented Feb 18, 2019

ljharb commented Feb 18, 2019

chenesan commented Feb 28, 2019

ljharb commented Mar 5, 2019

chenesan commented Mar 6, 2019 • edited

pgangwani commented Mar 6, 2019 • edited

chenesan commented Mar 6, 2019

pgangwani commented Mar 7, 2019 via email

ljharb commented Mar 7, 2019

pgangwani commented Mar 7, 2019

chenesan commented Mar 7, 2019

pgangwani commented Mar 8, 2019

chenesan commented Mar 9, 2019

pgangwani commented Mar 10, 2019

chenesan commented Mar 11, 2019

chenesan commented May 29, 2019 • edited

Acesmndr commented May 31, 2019

yuritoledo commented May 31, 2019

Acesmndr commented Jun 4, 2019 • edited

JonRCahill commented Jun 8, 2019 • edited

ljharb commented Jun 8, 2019

chenesan commented Jun 9, 2019 • edited

JonRCahill commented Jun 9, 2019

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenesan commented Jun 11, 2019

chenesan commented Jun 11, 2019

chenesan commented Jun 11, 2019

ljharb commented Jun 11, 2019

rodoabad commented Jul 30, 2019

chenesan commented Feb 7, 2019 •

edited

chenesan commented Mar 6, 2019 •

edited

pgangwani commented Mar 6, 2019 •

edited

chenesan commented May 29, 2019 •

edited

Acesmndr commented Jun 4, 2019 •

edited

JonRCahill commented Jun 8, 2019 •

edited

chenesan commented Jun 9, 2019 •

edited