Add ability to dive() and shallow-render-as-root Consumers and Providers by minznerjosh · Pull Request #1966 · enzymejs/enzyme

minznerjosh · 2019-01-05T16:00:24Z

Fixes #1958
Fixes #1908
Fixes #1647

I think the most controversial choice I've made here is only collecting context when a <Provider /> is the root (either because it is .dive()ed or passed directly to shallow(<Provider />)).

If we wanted to reliably make a <Consumer />'s value be that of the closest <Provider />, we'd need to recursively .dive() from the top until we find that <Provider />. I don't like that idea because we could end up rendering components that the end user did not want rendered.

I think, if the user does want all their <Provider />s' values to be automatically collected, that's where wrappingComponent (from #1960) will come into play.

minznerjosh · 2019-01-05T16:03:29Z

packages/enzyme-adapter-react-16.3/src/ReactSixteenThreeAdapter.js

-      render(el, context) {
+      render(el, context, {
+        providerValues = new Map(),
+      } = {}) {


I'd like to make the third argument an object. This way we can easily pass additional params as enzyme/react evolves without introducing breaking changes.

ljharb · 2019-01-07T03:31:00Z

enzyme doesn't yet have any support for createContext, so there shouldn't be any mention of "Consumer" or "Provider" in documentation until that's done.

Providers, prior to createContext being a feature, are components that provide child context.

minznerjosh · 2019-01-07T03:49:16Z

I'm a bit confused. There are a couple of tests (this one and this one) that specifically deal with the createContext() API. When mount()ing, the proper value gets passed down and everything—it functions fully! W/r/t shallow(), a component with a <Provider /> or <Consumer /> will render, but you currently can't .dive() it to make it render for real. So it seems like enzyme does have some support for createContext(). Once you are able to .dive() <Consumer />s and <Provider />s, as this PR implements, what more must enzyme do to support .createContext()?

I do think the gotcha about needing to .dive() a <Provider /> to have it affect a downstream <Consumer /> does need to be called out in documentation. Perhaps I could add some clarification that this regards the new createContext() API?

minznerjosh · 2019-01-08T15:42:54Z

@ljharb I've created #1973 to try to come to consensus on what full support for createContext() in enzyme should look like.

dschinkel · 2019-01-12T02:50:58Z

related reduxjs/react-redux#1161

minznerjosh · 2019-01-14T19:14:49Z

Btw I don’t think this implementation is quite right yet. It effectively auto-dives through as many Consumers/Providers as are necessary until it reaches a non Consumer/Provider. I think the correct behavior is for a Consumer/Provider to behave like any other component. Does that make sense?

I’ll take a look at implementing what I believe is the correct functionality in a couple of days.

ljharb · 2019-01-19T05:43:36Z

I definitely don't want a "diveUntil" API as part of shallow; that's something I'm planning to tackle as an entirely separate top-level API after we're caught up with React's featureset.

I'm not even confident that enzyme should auto-dive Providers and Consumers yet, to be honest.

minznerjosh · 2019-01-19T13:09:59Z

@ljharb I don't think we should auto-dive Providers/Consumers. I think instead you should be able to manually dive() them like any other component. I'm going to fix-up this PR today so that calling dive() once does not end up rendering more than one level of Consumer/Provider.

ljharb · 2019-01-19T17:30:26Z

@minznerjosh it makes sense that diving a Consumer/Provider should work as if it was a regular custom component with a function child - altho it's a bit surprising that it doesn't already do that without any enzyme changes.

dschinkel · 2019-01-22T17:47:40Z

@ljharb just wondering what you meant by:

Solving that problem is something I'm interesting in doing after we're caught up to React's featureset.

but then you also said:

I definitely don't want a "diveUntil" API as part of shallow; that's something I'm planning to tackle as an entirely separate top-level API after we're caught up with React's featureset.

I'm not even confident that enzyme should auto-dive Providers and Consumers yet, to be honest.

seems contradictory?

…ateContext()` providers and consumers - add `isContextConsumer` - add `getProviderFromConsumer`

…providers and consumers

minznerjosh · 2019-01-23T11:07:05Z

No worries! 😊 I was interpreting your earlier comments as “I’m not going to review this yet because there are major architectural/thematic flaws that need to be fixed before we get into the nitty-gritty of a code review.”

I’m hopeful this implementation does what you expect too! So now I’m going to chill out and wait for you to review. 😎

eugenet8k · 2019-02-07T18:33:08Z

Guys, any progress on submitting this PR? It is a big blocker for migration to react-redux v6 😭

hally9k · 2019-03-06T04:55:27Z

We too are blocked on this one, any update?

ljharb · 2019-04-02T21:10:20Z

@minznerjosh i've freshly rebased this. Can we make sure to add test cases for both shallow and mount to cover this functionality, including both consumers and providers as the root, or with .dive, .shallow, and .mount?

minznerjosh · 2019-04-02T21:13:20Z

@ljharb yup I'll give this PR some love tomorrow!

…`Provider` as root

minznerjosh · 2019-04-03T15:41:12Z

@ljharb Updated!

Added mount() coverage for rendering a <Provider />/<Consumer /> as root (there was already a spec for rendering them as not root)
The shallow() tests from the first iteration already had coverage for rendering both <Provider /> and <Consumer /> as root, and dive()ing them both
Added the required logic for wrappingComponent in shallow() to support createContext() (just like legacy context has to be pulled off the <RootFinder />, so does createContext() context)

Looking forward to your review!

ljharb

k, let's give this a shot :-)

- [new] `Utils` add `findElement`, `getNodeFromRootFinder`, `wrapWithWrappingComponent`, `getWrappingComponentMountRenderer`; add `RootFinder` (#1966) - [new] add `getDerivedStateFromError` support (#2036) - [fix] `shallow`/`mount`: `renderProp`: avoid warning when render prop returns `null` (#2076) - [build] include source maps - [deps] update `eslint` - [dev deps] update `eslint`, `semver`, `rimraf`, `react-is`, `html-element-map`, `chai`, `eslint-plugin-mocha`

- [new] support shallow rendering `createContext()` providers and consumers - add `isContextConsumer`, `getProviderFromConsumer` (#1966) - [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [refactor] use `react-is` predicates more - [deps] update `react-is` - [dev deps] update `eslint` - [build] include source maps

- [new] Add support for wrapping `Profiler` (#2055) - [new] support shallow rendering `createContext()` providers and consumers - add `isContextConsumer`, `getProviderFromConsumer` (#1966) - [new] add `wrapWithWrappingComponent`, `isCustomComponent` (#1960) - [new] add `getDerivedStateFromError` support (#2036) - [fix] avoid invariant violation in provider (#2083) - [fix] properly fix finding memo(SFC) components (#2081) - [fix] properly render memoized SFCs - [fix] `shallow`: avoid wrapping component for hooks - [deps] update `react-is` - [dev deps] update `eslint` - [refactor] use `react-is` predicates more - [build] include source maps

minznerjosh commented Jan 5, 2019

View reviewed changes

minznerjosh force-pushed the josh__dive-on-new-context branch from 041cdc0 to d2e3db0 Compare January 8, 2019 13:42

minznerjosh force-pushed the josh__dive-on-new-context branch from d2e3db0 to 6c2c3ac Compare January 8, 2019 23:41

markerikson mentioned this pull request Jan 12, 2019

Potential issues with store as a prop and shallow test rendering in v6 reduxjs/react-redux#1161

Closed

AdrienLemaire mentioned this pull request Jan 15, 2019

Dive shallow in a Context Consumer #1908

Closed

2 tasks

This comment has been minimized.

Sign in to view

minznerjosh force-pushed the josh__dive-on-new-context branch from 6c2c3ac to f627d1e Compare January 19, 2019 13:15

This comment has been minimized.

Sign in to view

minznerjosh added 2 commits January 22, 2019 14:50

[enzyme-adapter-react-{16,16.3}] [new] support shallow rendering `cre…

67e12eb

…ateContext()` providers and consumers - add `isContextConsumer` - add `getProviderFromConsumer`

[new] shallow: Support rendering and dive()ing createContext() …

7e12b15

…providers and consumers

minznerjosh force-pushed the josh__dive-on-new-context branch from f627d1e to 5566ebe Compare January 22, 2019 20:13

minznerjosh changed the title ~~Add ability to dive() Consumers and Providers~~ Add ability to dive() and shallow-render-as-root Consumers and Providers Jan 22, 2019

ljharb added API: shallow semver: minor New stuff. package: react adapter: 16 package: react adapter: 16.3 labels Jan 23, 2019

minznerjosh mentioned this pull request Jan 25, 2019

Add ability to mount()/shallow() a node inside a wrapping component #1960

Merged

ljharb mentioned this pull request Feb 16, 2019

dive() breaks when using the new context API. #1647

Closed

csvan mentioned this pull request Feb 19, 2019

Bring compatibility with v4 styled-components/jest-styled-components#191

Closed

OzzieOrca mentioned this pull request Mar 30, 2019

MHP-2233: Bring in react-native-testing-library and add utility funct… CruGlobal/missionhub-react-native#891

Closed

ljharb force-pushed the josh__dive-on-new-context branch from 7b08b1d to d35a5fa Compare April 2, 2019 21:10

[Tests] Add mount tests for rendering createContext() Consumer/…

7360912

…`Provider` as root

minznerjosh force-pushed the josh__dive-on-new-context branch from d35a5fa to 492cd0d Compare April 3, 2019 15:34

This comment has been minimized.

Sign in to view

ljharb force-pushed the josh__dive-on-new-context branch 2 times, most recently from e80e978 to 7360912 Compare April 4, 2019 06:53

ljharb approved these changes Apr 4, 2019

View reviewed changes

ljharb merged commit 2420e2b into enzymejs:master Apr 4, 2019

minznerjosh deleted the josh__dive-on-new-context branch April 4, 2019 20:47

ljharb mentioned this pull request Apr 5, 2019

How should enzyme support the createContext() API #1973

Closed

ljharb mentioned this pull request Apr 6, 2019

[Tests] add hooks tests #2008

Merged

7 tasks

christophehurpeau mentioned this pull request May 6, 2019

fix: shallow snapshots with decorators [no issue] ornikar/shared-configs#170

Merged

2 tasks

robertknight mentioned this pull request Jun 4, 2019

Support shallow rendering with new-style context preactjs/enzyme-adapter-preact-pure#52

Open

Uh oh!

Conversation

minznerjosh commented Jan 5, 2019 • edited by ljharb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minznerjosh Jan 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ljharb commented Jan 7, 2019

Uh oh!

minznerjosh commented Jan 7, 2019

Uh oh!

minznerjosh commented Jan 8, 2019

Uh oh!

dschinkel commented Jan 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minznerjosh commented Jan 14, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ljharb commented Jan 19, 2019

Uh oh!

This comment has been minimized.

minznerjosh commented Jan 19, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

ljharb commented Jan 19, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

dschinkel commented Jan 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minznerjosh commented Jan 23, 2019

Uh oh!

eugenet8k commented Feb 7, 2019

Uh oh!

hally9k commented Mar 6, 2019

Uh oh!

ljharb commented Apr 2, 2019

Uh oh!

minznerjosh commented Apr 2, 2019

Uh oh!

minznerjosh commented Apr 3, 2019

Uh oh!

This comment has been minimized.

ljharb left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

minznerjosh commented Jan 5, 2019 •

edited by ljharb

Loading

minznerjosh Jan 5, 2019 •

edited

Loading

dschinkel commented Jan 12, 2019 •

edited

Loading

dschinkel commented Jan 22, 2019 •

edited

Loading