[New] add enzyme-adapter-react-17

[layershifter]

Fixes #2429.

[layershifter]

This is the main change there, facebook/react#18377

[codecov]

Codecov Report

Attention: Patch coverage is 91.60156% with 43 lines in your changes are missing coverage. Please review.

Project coverage is 95.61%. Comparing base (cdfb1c6) to head (898978f).
Report is 88 commits behind head on master.

Files	Patch %	Lines
...pter-react-17/src/findCurrentFiberUsingSlowPath.js	68.42%	18 Missing ⚠️
...zyme-adapter-react-17/src/ReactSeventeenAdapter.js	95.90%	16 Missing ⚠️
...ges/enzyme-adapter-react-17/src/detectFiberTags.js	85.24%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2430      +/-   ##
==========================================
- Coverage   96.12%   95.61%   -0.52%     
==========================================
  Files          49       53       +4     
  Lines        4004     4516     +512     
  Branches     1123     1288     +165     
==========================================
+ Hits         3849     4318     +469     
- Misses        155      198      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ljharb]

There's a reason that issue did not have the "help wanted" label on it; thanks for the contribution, but I already have the majority of this change coded and am in the process of testing it.

Please leave this PR open so i can update it, otherwise the PR ref will be left orphaned.

[silver-snoopy]

Dear @layershifter thanks for the PR!

Any update on this?
React team has released V17 2 days ago, and It would be very much appreciated if this PR could go through

[wojtekmaj]

FYI, this might be affected by the bug reported in my temporary adapter repo:

wojtekmaj/enzyme-adapter-react-17#1

[layershifter]

Dear @layershifter thanks for the PR!

Any update on this?
React team has released V17 2 days ago, and It would be very much appreciated if this PR could go through

I stopped to work on it after @ljharb comment: #2430 (review). I am also interested in getting this.

[ljharb]

Indeed, that linked bug is one of the blockers I've run into in my local work.

I'd be more than happy to pull in additional commits to this PR, and to my local implementation, in service of getting something published ASAP.

[ljharb]

I've rebased this branch and included my WIP. There's still 8 failing tests (and 2 I've temporarily patched), that fall into these categories:

1. unknown node with tag 23. tag 23 is "OffscreenComponent" in the latest react-reconciler repo, but I have no idea how to render something useful here.
2. simulate issues in shallow: marked with "TODO: figure out why this is broken in shallow rendering in react 17"
3. issues because the getComponentStack function in enzyme-adapter-utils no longer creates an accurate error stack for React 17.

Please post links to commits/branches, and I'll pull them into the PR.

[eps1lon]

1. unknown node with tag 23. tag 23 is "OffscreenComponent" in the latest react-reconciler repo, but I have no idea how to render something useful here.

getLazyFiber("div").return.return.tag should do: https://codesandbox.io/s/offscreencomponent-tag-9hsq9

[ljharb]

@eps1lon awesome, thanks! the other side of this is, how do i locate, off of the FiberNode, what should be rendered? It's got a bunch of circular references. The best I could come up with makes this:

class Component extends React.Component {
        render() {
          return (
            <div>test</div>
          );
        }
      }

      const SuspenseComponent = () => (
        <Suspense fallback={Fallback}>
          <Component />
        </Suspense>
      );

map to this:

<SuspenseComponent>
  <Suspense fallback={[Function: Fallback]}>
    <Suspense mode="visible" />
  </Suspense>
</SuspenseComponent>

with this:

     case FiberTags.OffscreenComponent: {                                                                                                                                                                   
       console.log(node.return.memoizedProps.children);                                                                                                                                                     
       return {                                                                                                                                                                                             
         nodeType: 'function',                                                                                                                                                                              
         type: Suspense,                                                                                                                                                                                    
         props: { ...node.memoizedProps },                                                                                                                                                                  
         key: ensureKeyOrUndefined(node.key),                                                                                                                                                               
         ref: node.ref,                                                                                                                                                                                     
         instance: null,                                                                                                                                                                                    
         rendered: childrenToTree(nodeToHostNode(node.return.memoizedProps.children)),                                                                                                                      
       };                                                                                                                                                                                                   
     }                                                                                     

[eps1lon]

I don't have more context on how the OffscreenComponent is handled internally in React. From what I could gather it's used in different scenarios. Some only available with experimental APIs. If you can add a test I might be able to help.

[ljharb]

@eps1lon thanks! the test is already there; i've updated the branch to have my best output so far.

[eps1lon]

@ljharb
With

diff --git a/packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js b/packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js
index 0e69131..bb3477d 100644
--- a/packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js
+++ b/packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js
@@ -299,16 +299,7 @@ function toTree(vnode) {
     case FiberTags.Lazy:
       return childrenToTree(node.child);
     case FiberTags.OffscreenComponent: {
-      console.log(node.return.memoizedProps.children);
-      return {
-        nodeType: 'function',
-        type: Suspense,
-        props: { ...node.memoizedProps },
-        key: ensureKeyOrUndefined(node.key),
-        ref: node.ref,
-        instance: null,
-        rendered: childrenToTree(nodeToHostNode(node.return.memoizedProps.children)),
-      };
+      return toTree(node.child);
     }
     default:
       throw new Error(`Enzyme Internal Error: unknown node with tag ${node.tag}`);
diff --git a/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx b/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
index c77bf15..234da0b 100644
--- a/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
+++ b/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
@@ -1112,7 +1112,7 @@ describeWithDOM('mount', () => {
       }
     }
 
-    it.only('finds Suspense and its children when no lazy component', () => {
+    it('finds Suspense and its children when no lazy component', () => {
       class Component extends React.Component {
         render() {
           return (
@@ -1130,7 +1130,6 @@ describeWithDOM('mount', () => {
       const wrapper = mount(<SuspenseComponent />);
 
       expect(wrapper.is(SuspenseComponent)).to.equal(true);
-      console.log(wrapper.debug());
       expect(wrapper.find(Component)).to.have.lengthOf(1);
       expect(wrapper.find(Fallback)).to.have.lengthOf(0);
     });

the suspense tests are passing. npm run test:only is still failing on 2 component stack related tests though.

[gurufordy]

Hi all, firstly thanks for all your hard work on this project.

Has this PR stalled? I'm not quite sure what state it's in apart from it's failing code coverage right now.

I'm probably one of many people who rely on packages that use this under the hood and are not able to upgrade to React 17 (safely) until this gets updated.

https://xkcd.com/2347/

[igorpupkinable]

What are the options for developers and projects with thousands of Enzyme for React 16 tests? Is this worth any more waiting or better to migrate off Enzyme?

[ljharb]

@igorpupkinable unfortunately there isn't any alternative to enzyme for the majority of what enzyme provides.

I still intend to complete this task; I'm hoping next month.

[csvan]

@chyzwar @igorpupkinable RTL and Enzyme are VERY different, and people who migrate to RTL expecting to get feature parity with Enzyme are going to be sorely disappointed.

[createthis]

RTL and Enzyme are VERY different, and people who migrate to RTL expecting to get feature parity with Enzyme are going to be sorely disappointed.

This decision by the react team and airbnb team remind me a bit of the angular 2 -> 3 decision making process. It makes sense from a today and future development perspective, but it gives zero thought to existing infrastructure with thousands of established tests. It feels like that's your problem not mine. These kinds of decisions historically have not gone over super well with the community. Maybe it doesn't matter though. Time will tell.

[ljharb]

@createthis which decision are you referring to?

React decided they no longer wanted to have to consider enzyme users when changing React internals.

Airbnb decided they didn't want to maintain the package, and I've been doing so, unfunded, on my own, for many years.

Nobody has decided enzyme shouldn't or won't support React 17+, it's just that out of all the people who've complained about their company's codebases, ZERO of them have provided funding or programmer time to make it happen.

[createthis]

@createthis which decision are you referring to?

React decided they no longer wanted to have to consider enzyme users when changing React internals.

Airbnb decided they didn't want to maintain the package, and I've been doing so, unfunded, on my own, for many years.

Yes.

[ljharb]

To be clear though, while I certainly am not happy about those two companies' decisions here, the blame (if there must be "blame") is shared equally by every single user of the project (individuals and/or corporations) who isn't contributing time and/or money to supporting it.

Open Source is not an inexhaustible source of free labor, as much as I wish the world's legal and economic systems were set up for that to be feasible.

[createthis]

Agreed. It's all just js in the end. Anyone can contribute. I did my best to move the ball forward. I got stuck and blocked on the rebase issues. If anyone could unblock me I might be willing to try again.

That said, without major company support from fb or airbnb or another large company using the ecosystem, it may be an uphill battle not worth fighting. Sometimes it's better to just go with the flow.

[hodgef]

To be clear though, while I certainly am not happy about those two companies' decisions here, the blame (if there must be "blame") is shared equally by every single user of the project (individuals and/or corporations) who isn't contributing time and/or money to supporting it.

Open Source is not an inexhaustible source of free labor, as much as I wish the world's legal and economic systems were set up for that to be feasible.

While I sympathize with this statement, I would suggest that a maintainer should still ensure that critical issues are resolved in a timely fashion (such as people not being able to update React because this dep is holding them back). If this cannot be done in say 2 years, then it's normal for users to be considering alternatives.

I know what being a lone maintainer feels like (maybe not at this scale though!), but I think it wouldn't be helpful for me to blame users for my inability to maintain the project that I own. I'm the maintainer, after all. If there was a situation where I couldn't maintain my projects, I would put a disclaimer or archive the project for the reasons mentioned (lack of time, lack of funding, lack of help, etc). Until a firm commitment to fix/prioritization is made by the maintainer there will inevitably be a cloud of doubt and people will be asking for status and talking about alternatives, it's just normal.

Another thing I want to add: It's not because a project is OSS that people will help maintain the project. People use OSS mostly to ensure deps are peer-reviewed and do not contain malware. People will still ping the maintainer, because only the maintainer is expected to maintain the project.

Hopefully the React team/@gaearon and Enzyme can reach some solution (maybe a compatibility layer) as this project is undeniably useful to many.