add enzyme-adapter-react-17 #2564

eps1lon · 2022-08-02T20:44:41Z

Stacked on #2534 (Diff against #2534)

Compared to #2534:

less React 17 branching in tests (goal is 0 branching but branching for error stacks is probably fine)
some legacy code removed

TODO:
Open to discussing the remaining items @ljharb

callstack/componentstack
Would need to discuss if we want to normalize them across React versions. For the initial release I would lean towards being ok with different stacks between 16 and 17 since that's also what developers experience.
hide internal Suspense fibers
Green CI
code coverage hygiene

Closes #2429
Closes #2534
Closes #2430

in 17.x

UNSAFE_componentWillReceiveProps() on setContext()

lifecycle methods disabled according to test.

React.forwardRef under 17.

didn't come up with a workaround.

original PR, plus add an act() wrapper on shallow's simulate(), just to show it doesn't help.

codecov · 2022-08-02T20:54:43Z

Codecov Report

Base: 96.31% // Head: 95.99% // Decreases project coverage by -0.32% ⚠️

Coverage data is based on head (8649c0c) compared to base (538b0d8).
Patch coverage: 93.32% of modified lines in pull request are covered.

❗ Current head 8649c0c differs from pull request most recent head 8712dca. Consider uploading reports for the commit 8712dca to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2564      +/-   ##
==========================================
- Coverage   96.31%   95.99%   -0.33%     
==========================================
  Files          49       53       +4     
  Lines        4207     4716     +509     
  Branches     1130     1279     +149     
==========================================
+ Hits         4052     4527     +475     
- Misses        155      189      +34

Impacted Files	Coverage Δ
packages/enzyme-adapter-utils/src/Utils.js	`96.26% <ø> (ø)`
...pter-react-17/src/findCurrentFiberUsingSlowPath.js	`68.42% <68.42%> (ø)`
...zyme-adapter-react-17/src/ReactSeventeenAdapter.js	`96.07% <96.07%> (ø)`
...ges/enzyme-adapter-react-17/src/detectFiberTags.js	`100.00% <100.00%> (ø)`
packages/enzyme-adapter-react-17/src/index.js	`100.00% <100.00%> (ø)`
...pter-react-helper/src/getAdapterForReactVersion.js	`100.00% <100.00%> (ø)`
packages/enzyme/src/ShallowWrapper.js	`99.13% <100.00%> (+0.01%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ljharb · 2022-08-03T23:02:06Z

@eps1lon thanks! (but i wish you'd posted a branch link instead of opening a new PR - i now have three PRs to juggle (this, #2534, and #2430), which makes landing this update even harder)

ljharb · 2022-08-03T23:02:55Z

Re your discussion point, as long as component stacks are correct within a React major/minor, there's no reason they need to stay the same as those change.

eps1lon · 2022-08-04T06:42:43Z

@eps1lon thanks! (but i wish you'd posted a branch link instead of opening a new PR - i now have three PRs to juggle (this, #2534, and #2430), which makes landing this update even harder)

What do you need to juggle and how would a branch help? The existing 2 are replaced by this one. If the forked component stack tests are fine, then this PR is ready to land

ljharb · 2022-08-04T06:43:59Z

Every pull request has a permanent, eternal, undeleteable ref created on Github (that's not fetched by git by default, but can be opted into), so all three PRs must point to the same commit before landing any of them, or else any outliers will remain as orphaned refs for eternity.

eps1lon · 2022-08-04T07:15:53Z

Why are the other two PRs relevant here? They've been abandoned. The PR description clearly states that thus PR replaces them. How would I get CI to work without a PR?

Is there any change substance to discuss or do we want to spend more time on unrelated details? If I understood you correctly you have very limited time. But even if someone gives you a green PR that's not ok because?

ljharb · 2022-08-04T07:37:23Z

@eps1lon the PR refs are part of the github repo, and no PRs are truly abandoned when a maintainer can update them directly. Either way, repo management concerns are never irrelevant/unrelated. On all my projects, I strenuously avoid having any abandoned PRs, and I repurpose every PR to ensure that all PR refs point to something useful.

This PR isn't quite green - coverage is failing - but it's great that tests are passing. When I have time to review it, I will certainly do so, and I appreciate the contribution.

ljharb

Are createClass components no longer supported in React 17?

ljharb · 2022-08-04T07:37:53Z

.gitignore

+# vim
+**/*.swp
+*.swp


gitignore stuff for a specific editor belongs in your global gitignore, instead of in every repo you touch; please revert this.

ljharb · 2022-08-04T07:39:05Z

CONTRIBUTING.md

+
+In another terminal tab execute a specific test file for faster TDD test execution:
+```bash
+node_modules/.bin/mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js


Suggested change

node_modules/.bin/mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js

npx mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js

hardcoding node_modules should always be avoided.

ljharb · 2022-08-04T07:42:32Z

packages/enzyme/src/ShallowWrapper.js

+            && instance
+            && context
+          ) {
+            if (lifecycles.componentWillReceivePropsOnShallowRerender) {


can you elaborate on this lifecycle option?

This code corrects shallow()'s calling of componentWillReceiveProps() and UNSAFE_componentWillReceiveProps() under react 17.

-- https://github.com/enzymejs/enzyme/pull/2534/files#r687066582

ljharb · 2022-08-04T07:43:38Z

packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js

+    // React stores references to the Provider on a Consumer differently across versions.
+    if (Consumer) {
+      let Provider;
+      if (Consumer._context) {
+        ({ Provider } = Consumer._context);
+      }
+      if (Provider) {
+        return Provider;
+      }
+    }


In React 17+, when is Consumer._context not set?

Never. But there's a test specifically for passing something other than createContext so these guards are required. I'll remove the outdated comment.

eps1lon · 2022-08-09T15:27:33Z

Are createClass components no longer supported in React 17?

I'm not aware of any changes to createClass. How would I tell the tests to test for createClass support? I was under the impression that would be automatic just like with the other adapter tests

ljharb · 2022-08-09T19:32:45Z

I think I diffed the 16 to 17 adapter files, and saw some code related to createClass removed.

In theory yes, the tests should be automatic, so if they're passing then maybe it's fine.

eps1lon · 2022-08-09T19:40:38Z

packages/enzyme-adapter-react-17/src/ReactSeventeenAdapter.js

+}
+
+function isStateful(Component) {
+  return Component.prototype && Component.prototype.isReactComponent;


@ljharb You're probably referring to this change compared to 16 regarding create-react-class changes?

Older versions of create-react-class might not pass this duck-typing check but then they would also not pass the same check in React 17 itself:

https://github.com/facebook/react/blob/v17.0.2/packages/react-reconciler/src/ReactFiber.new.js#L220

https://github.com/facebook/react/blob/v17.0.2/packages/react-reconciler/src/ReactFiber.new.js#L457-L458

Thanks for confirming :-)

SwatiJadhav46 · 2022-08-17T03:41:58Z

@eps1lon Thanks for integrating this adapter. We are eagerly waiting for this to release, as it's a blocker for us in our project.

eps1lon · 2022-08-17T18:08:37Z

This PR isn't quite green - coverage is failing - but it's great that tests are passing.

@ljharb Is this something you want me to address? As far as I can tell this adapter has not less coverage than the 16 adapter. But since 16 adapter has less coverage than the average it's somewhat expected that coverage decreases. It looks to me that coverage target isn't really a target and more a check for changes (96.31% looks completely arbitrary to me).

ljharb · 2022-08-17T21:42:37Z

@eps1lon nah definitely not a requirement - just something that'd be nice to improve, if possible, until i have the time to get to the PR. Coverage targets are just minimums; the ideal is always 100% :-)

Prior99 · 2022-10-27T07:59:54Z

@ljharb We're in dire need to have support for react 17 and 18 in enzyme. Is there anything we could help with to speed things up?

aks- · 2024-02-27T17:50:10Z

Hi! I worked on trying to get adapter 17 to completion and out. Can you please take a look @ljharb? Here is the link

ljharb and others added 20 commits August 2, 2022 22:48

[New] add enzyme-adapter-react-17

e91c355

feat: add an adapter for React 17

0797375

add versions to CI

42fce4a

version updates

46de537

offscreencomponent wip

13111be

test TODOs

1337f75

Remove TODO_17 tags

07504db

Fix two failing tests by using less fragile syntax.

75a15f4

Fix another test with fragile matching.

658595a

componentWillReceiveProps and UNSAFE_componentWillReceiveProps removed

9625c9f

in 17.x

Fix shallow() so that it calls componentWillReceiveProps() and

7a54423

UNSAFE_componentWillReceiveProps() on setContext()

Forgot a test case. Call componentWillReceiveProps() even when shallow

938d10a

lifecycle methods disabled according to test.

displayNameOfNode gets a little wonky when used with React.memo and

bbab94b

React.forwardRef under 17.

punt on the simulate failure. I worked on this all day yesterday and

9a432a6

didn't come up with a workaround.

Make a few changes I saw https://github.com/wojtekmaj requested in the

1f48ca9

original PR, plus add an act() wrapper on shallow's simulate(), just to show it doesn't help.

Add tip from https://github.com/ljharb to CONTRIBUTING

ae9b14a

Restore suspense tests to the best of my ability.

935c84a

createReactClass and React.Component pass the same check

18d536f

Remove unused TODO_17

69f2846

Adjust displayname test to be invariant across React versions

5f2f0b1

eps1lon force-pushed the feat/support-react17-sebbie branch from a7a7d75 to 165d692 Compare August 2, 2022 20:48

eps1lon added 2 commits August 3, 2022 18:17

Fix displayNameOfNode for Memo components

9dfd35f

Fix isEmptyRender for SimpleMemoComponent

cc64090

eps1lon force-pushed the feat/support-react17-sebbie branch from 165d692 to cc64090 Compare August 3, 2022 16:17

eps1lon added 3 commits August 3, 2022 20:34

Ensure Suspense tests are the same in 16 and 17

762ac8c

Remove React 16 logic that is no longer covered by tests

cffae58

Remove no obsolete "supports*" checks when detecting Fiber tags

7f309a1

eps1lon marked this pull request as ready for review August 3, 2022 19:59

ljharb reviewed Aug 4, 2022

View reviewed changes

eps1lon added 3 commits August 9, 2022 17:04

node_modules/.bin -> npx

3b9b1e3

Revert .gitignore changes from base branch

103174d

Remove outdated comment

5418b9d

Simplify semver matcher for create-react-class

8649c0c

eps1lon commented Aug 9, 2022

View reviewed changes

Merge branch 'master' into feat/support-react17-sebbie

8712dca

ljharb force-pushed the master branch 3 times, most recently from 43eb75e to 39e6b1f Compare November 3, 2022 21:47

This comment was marked as spam.

Sign in to view

This comment was marked as abuse.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add enzyme-adapter-react-17 #2564

add enzyme-adapter-react-17 #2564

eps1lon commented Aug 2, 2022 •

edited

codecov bot commented Aug 2, 2022 •

edited

ljharb commented Aug 3, 2022

ljharb commented Aug 3, 2022

eps1lon commented Aug 4, 2022

ljharb commented Aug 4, 2022 •

edited

eps1lon commented Aug 4, 2022

ljharb commented Aug 4, 2022

ljharb left a comment

ljharb Aug 4, 2022

ljharb Aug 4, 2022

ljharb Aug 4, 2022

eps1lon Aug 9, 2022

ljharb Aug 4, 2022

eps1lon Aug 9, 2022

eps1lon commented Aug 9, 2022

ljharb commented Aug 9, 2022

eps1lon Aug 9, 2022

ljharb Aug 9, 2022

SwatiJadhav46 commented Aug 17, 2022

eps1lon commented Aug 17, 2022 •

edited

ljharb commented Aug 17, 2022

Prior99 commented Oct 27, 2022

This comment was marked as spam.

This comment was marked as abuse.

aks- commented Feb 27, 2024 •

edited by ljharb

	node_modules/.bin/mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js
	npx mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js

add enzyme-adapter-react-17 #2564

Are you sure you want to change the base?

add enzyme-adapter-react-17 #2564

Conversation

eps1lon commented Aug 2, 2022 • edited

codecov bot commented Aug 2, 2022 • edited

Codecov Report

ljharb commented Aug 3, 2022

ljharb commented Aug 3, 2022

eps1lon commented Aug 4, 2022

ljharb commented Aug 4, 2022 • edited

eps1lon commented Aug 4, 2022

ljharb commented Aug 4, 2022

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

eps1lon commented Aug 9, 2022

ljharb commented Aug 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SwatiJadhav46 commented Aug 17, 2022

eps1lon commented Aug 17, 2022 • edited

ljharb commented Aug 17, 2022

Prior99 commented Oct 27, 2022

This comment was marked as spam.

This comment was marked as abuse.

aks- commented Feb 27, 2024 • edited by ljharb

eps1lon commented Aug 2, 2022 •

edited

codecov bot commented Aug 2, 2022 •

edited

ljharb commented Aug 4, 2022 •

edited

eps1lon commented Aug 17, 2022 •

edited

aks- commented Feb 27, 2024 •

edited by ljharb