RFC: Rely less on React internals

[petegleeson]

👋 Hello! I am opening this issue to start a discussion about how the mount rendering strategy in Enzyme could be built more on top of React rather than using the internals of React.

The problem
When doing a full DOM render using mount, Enzyme directly works with instances of React virtual DOM nodes.

One example is the ReactSixteenAdapter, where it needs to return information about the virtual DOM so the core library can run queries and other operations against it. ReactSixteenAdapter's toTree function maps instances of VDOM nodes to an object that the Wrapper can use. This is an adaptation of logic that achieves a similar purpose in the react-test-renderer package.

Inside the core enzyme package there is a lot of logic to traverse and query the component tree. Very similar to the behaviour provided by ReactTestInstance. Additionally, utility functions like propsOfNode, typeOfNode and childrenOfNode also have some reliance on React internals.

The symptoms of this coupling are visible when Enzyme breaks due to React upgrades. Issues like #1553 require a large amount of work. New node types that are introduced by React need to be introduced in Enzyme.

A solution
Use what exists in react-test-renderer as a layer between Enzyme and React's virtual DOM. Instead of interacting directly with VDOM objects, Enzyme uses the API of ReactTestInstance objects. This decouples Enzyme from React internals and means that changes inside React are less likely to break Enzyme functionality.

Last week I spent some time spiking how this could work. What I came up with relies on two things:

1. A way of creating a ReactTestInstance from a component ref. At the moment, there is no exposed way to create a new instance of ReactTestInstance. Only methods inside the react-test-renderer package can do this.
2. A way of plugging in Wrappers to Enzyme. This would be a higher level of configuration than what is available with adapters at the moment. While this is pretty much opting out of reusing any logic currently in the core library, it means that all the tree traversal and querying can be based on what ReactTestInstance provides. Basically it would be a way to tell Enzyme to use my ReactTestRendererWrapper class instead of ReactWrapper. By using methods like findAll as well as properties like parent and children that exist of ReactTestInstance, I was able to significantly reduce the amount of logic that was needed to live in Enzyme.

I will raise a PR with my spike. It is very much WIP but most of the core Enzyme concepts are working.

What do you think?

I would love to get some feedback from contributors and maintainers of this library. I realise that this is a big change, but I think that the stability that can be gained is worthwhile.

[ljharb]

Conceptually, this sounds great - we absolutely want to be more robust, and rely on as close to zero internals as possible.

I'll take a look at #1649 as I work through my notifications backlog; appreciate the effort!