React toTree() handling mismatches

[gaearon]

Sorry for deleting the template, there are a few very specific problems I saw in the source.

• I'm not sure how I feel about Import findCurrentFiberUsingSlowPath from react-reconciler/reflection #1399. It gives a false sense of security but in practice can still break in patch versions. I explain the reasoning in a comment there—basically, it works by accident, and if react-dom internal logic changes you'd have to fork it again anyway because you'd need to support multiple versions.

• It seems like this call won't handle a fragment inside a root correctly. It will just skip over any but the first item. I'm not sure Enzyme API allows rendering an array or a <Fragment> as a root node but React does. (Note React's toTree() is also broken in this way—but I fixed it in Fix fragment and portal handling in toTree() facebook/react#12154.)

• If the above is right, I suspect that this call might also not work with fragments. It is totally legitimate to put a fragment inside a portal.

I haven't had time to verify these in detail because I'm in the middle of fixing React toTree() bugs myself. But I thought it might be helpful to report for further investigation.

[jquense]

I've probably exacerbated 2 & 3 with #1513 as well. All 4 branches are supposed to the same logic, I think that would be better signaled by stacked cases but eslint.

[aweary]

@gaearon moving the conversion in #1399 here

I agree, open to suggestions how to fix those :-) There was a proposal from Enzyme side for us to expose toTree in the test renderer. But even that didn’t end up being used.

I don't remember the context around why toTree didn't end up being used; right now the React 16 adapter only uses react-test-renderer/shallow and ReactDOM is used directly for the mount renderer. It could be that the test renderer wasn't sufficient to meet feature parity with the existing API for things like event simulation (cc @lelandrichardson).

We’re totally open to Enzyme taking more ownership over this (and potentially exposing toTree on ReactDOM in some special testing mode), but we should probably start “blessed” integration from the test renderer.

Exposing toTree on ReactTestUtils should be sufficient for generating the tree structure Enzyme currently consumes. There's also the question of whether there should be a blessed way to traverse the tree. Right now we have RSTTraversal. I opened facebook/react#9505 about this a while ago.

This might be a good opportunity for both teams to collaborate on an RFC for a better low-level testing interface. I'm not sure either team has the time/resources to commit though, which is partially why previous attempts have dropped off.

[jquense]

it's be really nice to have Enzyme use toTree from react, it be a lot less awkward to do stuff like: https://github.com/4Catalyzer/enzyme-webdriver/blob/master/src/tree-traversal.js where we need to copy/paste that logic :)

[ljharb]

(Related to #1778 / #1790)