enzymejs / enzyme

JavaScript Testing utilities for React
https://enzymejs.github.io/enzyme/
MIT License
19.96k stars 2.01k forks source link

Add contextType support #2554

Open pablopalacios opened 2 years ago

pablopalacios commented 2 years ago

@ljharb I've rebased #2507 , applied your suggestions and added test cases for dynamic context values. Unfortunately, the test which uses wrappingComponent does not pass, it keeps rendering the initial context value. The context is correctly updated in the RootFinder though, but it's not propagated down the tree.

I tried my best to understand what is happening but I don't understand yet how things work internally yet. If you have any hint to point me in the right direction, I'll gladly move this PR forward.

Thank you for all the great work :)

ljharb commented 2 years ago

@pablopalacios thanks, but opening an additional PR is hugely disruptive - now both PRs must stay open forever, manually kept in sync, until both can be landed. In the future, please post a link to your branch on the original PR, and I can pull in your changes.

ljharb commented 2 years ago

I'll take a closer look at test failures when I have time to do so.

codecov[bot] commented 2 years ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -24.46 :warning:

Comparison is base (3a7701c) 96.42% compared to head (092f6d4) 71.97%.

:exclamation: Current head 092f6d4 differs from pull request most recent head 658422b. Consider uploading reports for the commit 658422b to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2554 +/- ## =========================================== - Coverage 96.42% 71.97% -24.46% =========================================== Files 49 49 Lines 4332 4210 -122 Branches 1156 1132 -24 =========================================== - Hits 4177 3030 -1147 - Misses 155 1180 +1025 ``` | [Impacted Files](https://codecov.io/gh/enzymejs/enzyme/pull/2554?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enzymejs) | Coverage Δ | | |---|---|---| | [...enzyme-adapter-react-16/src/ReactSixteenAdapter.js](https://codecov.io/gh/enzymejs/enzyme/pull/2554?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enzymejs#diff-cGFja2FnZXMvZW56eW1lLWFkYXB0ZXItcmVhY3QtMTYvc3JjL1JlYWN0U2l4dGVlbkFkYXB0ZXIuanM=) | `95.49% <100.00%> (-0.32%)` | :arrow_down: | ... and [29 files with indirect coverage changes](https://codecov.io/gh/enzymejs/enzyme/pull/2554/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enzymejs) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enzymejs). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=enzymejs)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

pablopalacios commented 2 years ago

@ljharb I can close this PR and post the branch in the other one, no problem. Regarding applying the changes in the 16.3 adapter: does it make sense? React just added support for contextType in version 16.6 (see: https://reactjs.org/blog/2018/10/23/react-v-16-6.html) or am I missing something?

ljharb commented 2 years ago

Please don't; once a PR is opened its a permanent ref on the repo, so it needs to stay open until it's merged.

ljharb commented 2 years ago

hmm, if contextType is only added in 16.6 then you’re right, it should only go in the 16 adapter

pablopalacios commented 1 year ago

@ljharb I finally had time to take a look a this one again. I was able to make the missing test to pass. Could you review it again?

ljharb commented 1 year ago

@pablopalacios i rebased everything and made a few fixes; looks like tests are failing on react ^16.6.

pablopalacios commented 1 year ago

@ljharb the tests are expected to fail since they use a version of react-shallow-renderer that does not have support for contextType. We need to merge this PR first.

ljharb commented 1 year ago

gotcha, thanks, i'll take a look at that one.

ljharb commented 1 year ago

@pablopalacios actually enzyme doesn't use react-shallow-renderer at all. Is there no way to fix it prior to taking on that dependency?

pablopalacios commented 1 year ago

@ljharb it depends on react-test-renderer which depends on react-shallow-renderer.

ljharb commented 1 year ago

We use v16 of react-test-renderer which does not depend on that - only v17+ does.