Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] Regression on CDU on setProps on shallow wrapped component #2027

Merged
merged 2 commits into from Mar 15, 2019
Merged

[TEST] Regression on CDU on setProps on shallow wrapped component #2027

merged 2 commits into from Mar 15, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2019

For #2020.

@ljharb
Copy link
Member

ljharb commented Mar 9, 2019

Thanks! I've updated the tests and split out the "no state change" and "state change" cases, and added some more assertions.

@ljharb
Copy link
Member

ljharb commented Mar 9, 2019

I suspect this is a bug in react's shallow renderer - I've filed facebook/react#15075 to investigate.

@ljharb
Copy link
Member

ljharb commented Mar 12, 2019

I rebased this on top of #1981, which seems related, but one of the shallow tests still fails.

@ghost
Copy link
Author

ghost commented Mar 12, 2019

Added describeIfs to ensure the tests only run for 16.3and up

@ljharb
Copy link
Member

ljharb commented Mar 12, 2019

Good catch :-) we still need to make the shallow test pass, though. Got any ideas for a fix?

@ghost
Copy link
Author

ghost commented Mar 13, 2019

The new commit satisfies the failing test, but are there any other conditions/test cases we should cover?

@ljharb
Copy link
Member

ljharb commented Mar 15, 2019

Probably :-) but if all the tests are passing, and it looks reasonable, let's merge it!

@ghost
Copy link
Author

ghost commented Mar 15, 2019

Alrighty 👍

@ljharb ljharb merged commit ebdd4e7 into enzymejs:master Mar 15, 2019
@ghost ghost deleted the 2020/cdu-on-setprops branch March 15, 2019 17:23
@dallonf
Copy link

dallonf commented Mar 27, 2019

What would it take for this fix to get released to NPM? I think I just got bitten by this bug when updating dependencies

@ljharb
Copy link
Member

ljharb commented Mar 27, 2019

I'm waiting on a few more PRs to land before publishing the next semver-minor of enzyme itself.

@bdwain
Copy link
Contributor

bdwain commented Apr 1, 2019

@ljharb are you still waiting for more versions? It would be nice to get bugfixes out ASAP since the latest release is presumably broken. I'd be happy to do a branch of 3.9.0 with just this if it would get released as 3.9.1

@ljharb
Copy link
Member

ljharb commented Apr 2, 2019

I might be able to pull together a patch tomorrow; but there’s still a minor I’m hoping to land first.

@jdk243
Copy link

jdk243 commented Apr 22, 2019

@ljharb any estimate of when this patch is going to make it to NPM? getting some test failures related to this issue after updating to 3.9.0

@joshmanders
Copy link

Is there any update on this? This is breaking our tests.

@cedosw
Copy link

cedosw commented May 17, 2019

Same here ... tests are failing because of this :(

@bdwain
Copy link
Contributor

bdwain commented May 21, 2019

@ljharb seems like releases of things are going out, but not the main enzyme package itself. Any reason why there can't be a release? The latest version has been broken for 2 months while master has been fixed. Seems like it shouldn't be terrible to release a patch version of whatever is in master.

@sulemanof
Copy link

Really need this fix to be released 😢

@joshmanders

This comment has been minimized.

@ljharb

This comment has been minimized.

@ljharb
Copy link
Member

ljharb commented May 24, 2019

@bdwain every main enzyme release potentially creates a flurry of new issues, so it is in fact quite terrible to release it before it's ready. I'm still planning to get a few fixes in before releasing v3.10.

@paulkmiller

This comment has been minimized.

@zackify

This comment has been minimized.

@ljharb

This comment has been minimized.

@joshmanders

This comment has been minimized.

@enzymejs enzymejs locked as too heated and limited conversation to collaborators May 24, 2019
@ljharb
Copy link
Member

ljharb commented Jun 4, 2019

v3.10.0 has now been released.

@enzymejs enzymejs unlocked this conversation Jun 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet