Support all Component Lifecycle methods in ShallowRenderer

[koba04]

This is a feature request which is supporting all React Component Lifecycle methods.
This is including some breaking changes.

This implementation is working on progress, and no (tests is passed)

Does this make sense?
If you are interested in this PR, please give me your feedbacks.
I'd like to work on it 😄

Thanks.

[lelandrichardson]

@koba04 This is really cool work. I had briefly considered something similar to this a while ago but never pursued it. I like this idea in general, but let me voice a couple of things concerning me:

1. refs need to work correctly, and I don't think they will out of the box. We would probably have to write some code to handle this as well.
2. context will also have to work, which it looks like is not included in this PR yet.
3. does this work how you'd expect when this.setState is called from with inside the lifecycle methods? I didn't see any tests covering this yet
4. we have to ensure that the lifecycle methods are called properly, in the proper order, and with the proper level of asynchrony. Is componentDidMount called in the same tick that componentWillMount is called in? I think it is, but we should make sure.

This PR would be a big breaking change and would push us to 3.0. As a result, I might choose to sit on it for some amount of time, as we had a couple of other things planned for 3.0. This is great work though and would be a welcome addition.

[koba04]

Thank you for your great feedbacks 😄

refs need to work correctly, and I don't think they will out of the box. We would probably have to write some code to handle this as well.

yeah, refs doesn't work in this implementation because Shallow Rendering doesn't support it yet.
I think supporting refs is the other issue. But I'm looking for the way refs works well.

context will also have to work, which it looks like is not included in this PR yet.

right. I'll add tests for context and try to work it well if it doesn't work.

does this work how you'd expect when this.setState is called from with inside the lifecycle methods? I didn't see any tests covering this yet

I'll add tests for when setState is called in lifecycle methods. I guess it works well except batchingUpdate feathers.
In lifecycle methods, setState calls are batched.
I guess it might be able to support it using ReactDOM.unstable_batchedUpdates.

we have to ensure that the lifecycle methods are called properly, in the proper order, and with the proper level of asynchrony. Is componentDidMount called in the same tick that componentWillMount is called in? I think it is, but we should make sure.

I guess so. I'll confirm the original behaviors of React.

Currently, this PR is including very minimum implementations.
I'll consider the implementation makes more precisely using React internal APIs like ReactUpdates.

[jwbay]

This would be fantastic. My shallow rendering tests are chock full of x.instance().componentDidMount(). Would ref support be a blocker for this PR getting released considering the underlying shallow renderer from React just doesn't support them?

[koba04]

I think ref support is not a blocker for this PR.

Because it is a limitation of Shallow rendering. Current shallow implementation is not supporting it too.

I might be able to add the refs support in this PR. But it would be better to implement it in React Shallow rendering instead of including it in enzyme.

This PR is in an early stage. I have to solve some issues and write some test cases.

[aweary]

@koba04 any progress on this since? I'm super interested in seeing this merged

[koba04]

@aweary
I think this PR has the following problems.

• Support Refs. (must?)
• Support Context
• Support to call setState in Lifecycle methods. (I already created the PR for this ShallowRenderer.simulate supports batched updates #342.)
• Ensure that the lifecycle methods are called properly, in the proper order, and with the proper level of asynchrony. (I have to confirm this but I guess it is implemented properly already.)

I think this can be merged if #342 is merged and I've ensured that the lifecycle methods are called properly.

[aweary]

@koba04

Is #342 required for setState to work in specific lifecycle methods? It seems like setState works in componentWillReceiveProps as is, so what does batching provide that enables setState in other methods?

Most lifecycle methods are invoked in ReactCompositeComponent.updateComponent with componentDidMount being enqueued in a transaction and componentWillMount being called in performInitialMount so I think what you have here is pretty close as far as order goes. How accurate does the asynchrony need to be? I think if we just follow this diagram from the React source we should be mostly fine.

/**
 * ------------------ The Life-Cycle of a Composite Component ------------------
 *
 * - constructor: Initialization of state. The instance is now retained.
 *   - componentWillMount
 *   - render
 *   - [children's constructors]
 *     - [children's componentWillMount and render]
 *     - [children's componentDidMount]
 *     - componentDidMount
 *
 *       Update Phases:
 *       - componentWillReceiveProps (only called if parent updated)
 *       - shouldComponentUpdate
 *         - componentWillUpdate
 *           - render
 *           - [children's constructors or receive props phases]
 *         - componentDidUpdate
 *
 *     - componentWillUnmount
 *     - [children's componentWillUnmount]
 *   - [children destroyed]
 * - (destroyed): The instance is now blank, released by React and ready for GC.
 *
 * -----------------------------------------------------------------------------
 */

[koba04]

@aweary Thank you for your feedbacks!

I think the following cases should be batching updates.

• componentWillMount and componentDidMount when a component is mounted.
  • https://jsfiddle.net/koba04/s3pmr7yz/1/
• componentWillReceiveProps and componentWillUpdate and componentDidUpdate when a component is updated.
  • https://jsfiddle.net/koba04/ruaz8eu2/ (componentWillUpdate)
  • https://jsfiddle.net/koba04/y4syhyyL/1/ (componentDidUpdate)
  • https://jsfiddle.net/koba04/3yg25mwm/1/ (componentWillReceiveProps)

For supporting the above, I'm going to use #342 .
The test cases are 224d320

[aweary]

Got it, so that PR should make setState work as it does internally, which in turn allows the lifecycle methods to behavior as we'd expect 👍

As far as execution order goes this seems generally in line with ReactCompositeComponent, so ref support seems to be the major blocking item for this specific PR.

[koba04]

Yeah, I'm considering to support Refs. I think I have two options for it.

• Simulating Refs behaviors in enzyme.
• Adding Refs support in React Shallow.

[erikthedeveloper]

This is great to see someone ( @koba04 ) taking this on! We have been also performing similar hacks (i.e. instance.componentDidUpdate(prevProps) etc...) all over our tests. I'm not 100% clear on what steps remain here (is this PR blocked by ref support?), but I am open to contribute.

[lelandrichardson]

The reason ref support is a big deal is actually part of a broader issue than just refs. The problem is that there are certain lifecycle methods where the existence of backing DOM nodes is expected (and thus, React.findDOMNode should return something, and refs should return instances.

Historically shallow has been something that systematically stayed away from these lifecycle methods because when you start expecting backing instances of child components to exist, the whole point of shallow goes out the window.

Adding in full lifecycle support for components that don't use refs or React.findDOMNode makes a lot of sense, but probably only for these cases.

If we could somehow make this API an opt-in feature, then I don't think support for refs would be needed at all.

One thing we could do starting off is provide it as an "experimental" flag:

const wrapper = shallow(<Foo />, { lifecycleExperimental: true })

or something like that.

@koba04 what are your thoughts?

[ljharb]

SECRET_LIFECYCLE_DO_NOT_USE_OR_YOU_WILL_BE_FIRED

[lelandrichardson]

more like EXPERIMENTAL_LIFECYCLE_DO_NOT_GET_MAD_AT_US_IF_BEHAVIOR_CHANGES :)

[koba04]

@lelandrichardson

One thing we could do starting off is provide it as an "experimental" flag:
const wrapper = shallow(, { lifecycleExperimental: true })

I think it is a good first step for supporting lifecycle methods!
It would be nice if this is added with the experimental flag at first.

I agree with supporting ref is a big deal.
It's a limitation of ShallowRendering.

https://facebook.github.io/react/docs/test-utils.html#shallow-rendering

I think supporting ref is an issue of the shallow rather than lifecycle methods in shallow.
Because, when we use shallow, we can't use ref in your component's render method even now.

I'd like to take this too, but I'd like to take as an another issue.

[koba04]

Historically shallow has been something that systematically stayed away from these lifecycle methods because when you start expecting backing instances of child components to exist, the whole point of shallow goes out the window.

Right, Supporting ref needs to instantiate child components, which is including to call lifecycle methods of child components.
If the child component has ref for using its child component instances, should we instantiate it?

I think it's no longer shallow...

We need to discuss how ref should behave in shallow.

[koba04]

interesting.

Ben has been prototyping a full React test renderer that can render everything both on React Native and DOM.
Unlike shallow renderer, it is stateful and renders deeply.

https://github.com/reactjs/core-notes/blob/master/2016-05/may-12.md#snapshot-testing

[jwbay]

Looks like there might be another renderer for Enzyme to wrap 😅

As described, I don't think the new test renderer would make shallow rendering any less valuable. If there was a switch or option to turn off deep rendering, though, that would be interesting.

[aweary]

If anything it seems to be closer to mount, though I guess we'll see 👍 it might make things a lot easier for us. It'd be nice if we could see what progress has been made with it, or what the API might look like.

@spicyj how do you think this new renderer might affect enzyme?

[sophiebits]

Yes, it's like mount except you don't need jsdom and it would also work for RN components without a host environment. It just outputs JSON.

[aweary]

@spicyj very interesting, thanks for chiming in. JSON should be just about as easy as it gets for us in terms of consuming/wrapping the API. Sounds like a great middle-ground between shallow and mount, looking forward to seeing more 👍

[koba04]

@aweary @lelandrichardson

I've implemented lifecycleExperimental flag for enabled this feature.

To be clear, the following tables are differences of behavior by the flag.

mounting phase

lifecycleExperimental	true	false
componentWillMount	◯	◯
componentDidMount	◯	×

updating phase

setState

lifecycleExperimental	true	false
shouldComponentUpdate	◯	◯
componentWillUpdate	◯	◯
componentDidUpdate	◯	◯

setProps and setContext

lifecycleExperimental	true	false
shouldComponentUpdate	◯	◯
componentWillReceiveProps	◯	◯
componentWillUpdate	◯	◯
componentDidUpdate	◯	×

unmounting phase

lifecycleExperimental	true	false
componentWillUnmount	◯	◯

[nfcampos]

@koba04 could we have a test asserting the order in which lifecycle hooks are called, especially those related to updates?

[koba04]

@thatguynamedandy It's fixed in 2.4.1. Thank you for reporting it!

[aweary]

Thanks for all the great work @koba04 🎉

[blainekasten]

Did we ever remove the lifecycleExperimental flag and just make it part of the standard flow?

[koba04]

I've created a PR for React supports all lifecycle methods in ShallowRender.

• Add support all lifecycle methods to ShallowRender facebook/react#7912

If the PR is merged, we can remove this implementation and enable this without lifecycleExperimental flag.

[ljharb]

@koba04 we'd still need to support React 0.13, 0.14, and v15, so we'd have to think carefully about how we maintain compat across all React versions.

[koba04]

@ljharb Sure. That PR has a breaking change, so I think these are possible to exist together by using the REACT16 variable.

[nevir]

Does facebook/react#9426 allow it to be enabled by default?

[koba04]

@nevir No, ShallowRenderer v16 will behave the same way.
https://github.com/facebook/react/pull/9426/files#diff-a8123b15946bc2724f8ff58d87aa3bbbR183

I think it would be nice if ShallowRenderer supported this in a future major release.

[nazreen]

thank you @koba04 for this PR! just read through the whole thread simply because I was curious of the culmination of this feature

[koba04]

@nxmohamad
Thanks!
In React v16, ShallowRenderer never calls componentDidMount and componentDidUpdate so we need some works to support all lifecycle methods for React v16.

[jose920405]

hi @koba04

Thank you very much for the work done, it has been very helpful.

With this last comment you put, I finally make sense of a problem I'm going through #1279.

My Info:

"name": "enzyme",
"version": "3.1.0",

---

"react": "16.0.0",
"react-native": "0.47.2"

componentDidUpdate not works for me.

I am testing and apparently this is a problem related to redux interactions. When I change my redux dispatch by a simple setState, componentDidUpdate is executed correctly.

In case this is not yet available for React v16. I wanted to ask if you or someone is currently working on this?

[koba04]

Hi, @jose920405

I am testing and apparently this is a problem related to redux interactions. When I change my redux dispatch by a simple setState, componentDidUpdate is executed correctly.

According to this comment, are you using lifecycleExperimental flag, aren't you? What about componentWillUpdate?
If so, that is an unexpected behavior. Can you create a reproduce case for this?

Currently, enzyme has a bug that doesn't call componentDidUpdate for setState if you don't enable lifecycleExperimental. This is a PR to fix this #1261.

[jose920405]

Yeah, i'm using lifecycleExperimental. Although I understand, for version 3.1.0 is already default to true. Anyway I added it to be sure.

componentWillUpdate is not fired either

My code:

My LoginScreen Component

// Reducer
import UserReducer from '../Redux/Reducers/UserReducer'

class LoginScreen extends Component {

  constructor(props) {
    super(props)
  }

  componentDidMount() {
    console.log('componentDidMount ===>')  // This log is fired
  }

  componentWillUpdate() {
    console.log('componentWillUpdate ====> ')  // This log is never fired
  }

  componentDidUpdate() {
    console.log('componentDidUpdate ====> ')  // This log is never fired
  }

  render() {
    console.log('Inside component Render')  // This log is fired
    return (
      <View style={styles.mainContainer}>
        <View style={styles.formContainer}>
          <TextInputForm
            eachFormCustomTop={styles.eachFormCustomTop}
            icon={{
              source: Images.iconLogin
            }}
            textInput={{
              style: styles.textInputs,
              placeholder: 'Email',
              autoCapitalize: 'none',
              keyboardType: 'email-address',
              value: this.props.user.email,
              onChangeText: (text) => {
                console.log('onChangeText email text ==> ', text); // this log fires.
                this.props.email_typing(text) // `componentDidUpdate` doesn't wors

                // this.setState({ // If i use `setState` works
                //   changeTest: ''
                // })

              },
            }}
          />
        </View>
      </View>
    )
  }
}

const mapStateToProps = (state) => {
  return {
    user: state.user
  }
}

const mapDispatchToProps = (dispatch) => {
  return {
    email_typing: (text) => dispatch(UserReducer.email_typing(text)),
  }
}

export default connect(mapStateToProps, mapDispatchToProps)(LoginScreen)

My Test File:

import React from 'react'

import { createStore, combineReducers } from 'redux';

import Enzyme, { shallow, mount } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';

Enzyme.configure({ adapter: new Adapter() });

import LoginScreen from '../../App/Containers/LoginScreen'

// Components
import TextInputForm from '../../App/Components/TextInputForm'

const store = createStore(
  combineReducers(
    {
      user: require('../../App/Redux/Reducers/UserReducer').reducer,
    }
  )
);

function shallowComp() {
  return shallow(
    <LoginScreen />,
    { context: { store }, lifecycleExperimental: true },
  );
}

function updateComp() {
  render = wrapper.update().dive();
}

let wrapper = shallowComp();
let render = wrapper.dive();

describe('Login Screen Test', () => {

  test('>>>>> Render Correctly', () => {
    expect(wrapper).toHaveLength(1)
  })

  test('>>>>> onChangeText in Email', () => {
    const textInputs = render.find(TextInputForm);
    const emailTextInput = textInputs.first().getElement().props.textInput;
    
    emailTextInput.onChangeText('[email protected]') // Here Execute `onChangeText` inside render of LoginScreen comp

    updateComp()

    const textInputsChanged = render.find(TextInputForm);
    let emailTextInputChanged = textInputsChanged.first().getElement().props.textInput;
    expect(emailTextInput.value).not.toEqual(emailTextInputChanged.value)
  })
})

this.props.email_typing(text) inside onChangeText:

It works as expected, runs the change on my reducer, and causes the render to run again. componentDidUpdate DOES NOT FIRE.

and as I mentioned earlier, running setState works perfectly.

[koba04]

@jose920405
Thank you for your reproduce code! In React v16, ShallowRenderer's setState doesn't call componentDidUpdate, so I've added to support this but this is called only through a ShallowWrapper instance enzyme provides. So if you call setState of React Component instance directly, componentDidUpdate is never called. I'm working on to fix this, but this seems to need to override setState of the component instance. I'm not sure it's a right way to fix this so I'll keep to working on.

koba04@88fdddc

[jose920405]

@koba04

When you say.

if you call setState of React Component instance directly, componentDidUpdate is never called.

This is something that is not clear to me, since it works very well for me.

If we go to my example inside onChangeText and replace this.props.email_typing(text) to:

this.setState({ // If i use `setState` works
   changeTest: 'anyval'
})

results in the componentDidUpdate entering correctly.

My problem is directly with redux actions and not with setState.
Although I suppose at the end of the day, a redux action must run a setState at some point

[koba04]

@jose920405 Thanks, That's a weird behavior. Could you create a repository or gist to be able to confirm this?

[jose920405]

hi @koba04

apologies for the delay, I was quite busy in other things these days 🙂

This is the repo with the example.

https://github.com/jose920405/example-react-native-redux-with-enzyme

unfortunately I have not been able to replicate what I say to you if the componentDidUpdate works for me when I use the setState 🙁 .

But I'm still testing it on my original project and there if I still use the componentDidUpdate 😂 , there's something I should be doing that I may not remember.

This is my proving:

---

For now it is happening to me what it says that the componentDidUpdate does not run.

I wanted to thank you for your work in this functionality.

I also wanted to ask you about your progress and if you have a close estimate of when you can be ready. I look forward to this work to continue my test.

[koba04]

@jose920405 Thanks for creating the repo!
Currently, I have no time for this so no I'm not sure what I should do for this yet.
But I'll keep working on this with the repo. Thanks!