mul53 commented 3 years ago

Fixes #2258

Below are two code sandboxes that show the bug exists, i added enzyme to the sandbox when you run the tests sandbox 2 doesn't call the method while sandbox 1 does.

Sandbox #1 enzyme 3.3.0, react 16.14.0
Sandbox #2 enzyme 3.4.0, react 16.0.0

codecov[bot] commented 3 years ago

Codecov Report

Merging #2473 (63a4054) into master (c68aff6) will increase coverage by 0.10%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2473      +/-   ##
==========================================
+ Coverage   96.10%   96.20%   +0.10%     
==========================================
  Files          49       49              
  Lines        3873     3904      +31     
  Branches     1085     1091       +6     
==========================================
+ Hits         3722     3756      +34     
+ Misses        151      148       -3

Impacted Files	Coverage Δ
...e-adapter-react-16.1/src/ReactSixteenOneAdapter.js	`94.86% <100.00%> (+0.34%)`	:arrow_up:
...e-adapter-react-16.2/src/ReactSixteenTwoAdapter.js	`94.88% <100.00%> (+0.32%)`	:arrow_up:
...adapter-react-16.3/src/ReactSixteenThreeAdapter.js	`95.25% <100.00%> (+0.27%)`	:arrow_up:
...enzyme-adapter-react-16/src/ReactSixteenAdapter.js	`95.31% <100.00%> (+0.16%)`	:arrow_up:
packages/enzyme/src/Utils.js	`95.54% <0.00%> (+0.69%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c68aff6...c68aff6. Read the comment docs.

mul53 commented 3 years ago

@ljharb should i also add the patch for adapter versions 16.1-3, i see the tests are failing on those or keep for that specific version

ljharb commented 3 years ago

@mul53 yep! however, if this PR is otherwise good to go, I'll go ahead and backport the fixes for you; if not, I'll leave it for you :-)

(also, in the future, you may not want to make PRs from master, so things are easier on other fellows making PRs from the same fork, but nbd)

mul53 commented 3 years ago

@ljharb Here are the sandboxes

React 0.14 Sandbox React 15 Sandbox React 16.2 Sandbox React 16.3 Sandbox React 16.4 Sandbox

I will continue looking at the issue

ljharb commented 3 years ago

hmm, that was not intentional

ljharb commented 3 years ago

@mul53 i think you'll need to create a new PR from 63a40541b5c34e689633428fde2599c925ba56e5 that's from a branch that's not master, since the MLH fork has the pull bot set up on it that's messing things up :-/

mul53 commented 3 years ago

@ljharb I will do so, there are tests to fix

enzymejs / enzyme

Fix: setContext doesn't trigger componentWillReceiveProps #2473

Codecov Report