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

Shallow renders though portal child #1761

Merged
merged 2 commits into from
Aug 18, 2018

Conversation

jgzuke
Copy link
Collaborator

@jgzuke jgzuke commented Aug 16, 2018

elementToTree when given a Portal element returns a wrapper node and renders through children. Previously this would just return the element itself which would show in debug as <Portal /> but not render through any children.

@jgzuke jgzuke requested a review from ljharb August 16, 2018 22:50
if (el.$$typeof === Portal) {
return {
nodeType: nodeTypeFromType(Portal),
type: Portal,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jgzuke jgzuke force-pushed the jgzuke-support-portal-shallow branch from 4854d43 to 9cf359f Compare August 16, 2018 23:01
@@ -94,6 +97,10 @@ export function displayNameOfNode(node) {

if (!type) return null;

if (type === Portal) {
return 'Portal';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as on the other PR; should we add this here, or only on the adapters?

@@ -168,9 +178,20 @@ export function ensureKeyOrUndefined(key) {
}

export function elementToTree(el) {
if (el === null || typeof el !== 'object' || !('type' in el)) {
if (el === null || typeof el !== 'object' || !('type' in el || el.$$typeof === Portal)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the $$typeof part should definitely not be here; that should be adapter-specific.

@jgzuke jgzuke force-pushed the jgzuke-support-portal-shallow branch from 6ef7f24 to 5d24796 Compare August 17, 2018 20:18
@jgzuke
Copy link
Collaborator Author

jgzuke commented Aug 17, 2018

@@ -180,7 +180,9 @@ export function ensureKeyOrUndefined(key) {
return key || (key === '' ? '' : undefined);
}

export function elementToTree(el) {
export function elementToTree(el, wrapperElementToTree = null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb is there a nicer way to do this? One option would be to replace elementToTree entirely in the 16+ adapters, but we can't just replace the top level calls and fall through like displayNameOfNode and nodeTypeFromType since this is recursive

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. i guess this could be like recurse = elementToTree, and then use recurse below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default second argument would break .map(elementToTree) since that provides a second index arg. Im not sure if its nicer to switch the .maps or leave this as typeof recurse === 'function'

}

function elementToTree(el) {
if (!(el && el.$$typeof === Portal)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this line can be if (!isPortal(el)) {?

}

function elementToTree(el) {
if (!(el && el.$$typeof === Portal)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also isPortal here?

}

function elementToTree(el) {
if (!(el && el.$$typeof === Portal)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPortal

@@ -78,6 +78,33 @@ function flatten(arr) {
return result;
}

function nodeTypeFromType(type) {
if (type && type === Portal) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if type is falsy, it won't === Portal, so i think this can be type === Portal ? 'portal' : utilNodeTypeFromType(type)

@@ -180,7 +180,9 @@ export function ensureKeyOrUndefined(key) {
return key || (key === '' ? '' : undefined);
}

export function elementToTree(el) {
export function elementToTree(el, wrapperElementToTree = null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. i guess this could be like recurse = elementToTree, and then use recurse below?

@jgzuke jgzuke force-pushed the jgzuke-support-portal-shallow branch from 73a2aa0 to 61ef730 Compare August 17, 2018 23:24
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great.

@ljharb ljharb force-pushed the jgzuke-support-portal-shallow branch 2 times, most recently from 0982fbc to 892d368 Compare August 18, 2018 04:07
@ljharb ljharb merged commit 5774d0d into enzymejs:master Aug 18, 2018
ljharb added a commit that referenced this pull request Aug 24, 2018
 - createMountWrapper: add Portals to propType (#1760, #1761, #1772)
 - add pointer events (#1753)
 - `elementToTree`: add ability to inject recursed `elementToTree` (#1760)
ljharb added a commit that referenced this pull request Aug 25, 2018
 - [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke)
 - [New] Add pointer events support (#1753, @ljharb)
 - [New] Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense)
 - [New] pass the adapter into `createMountWrapper` (#1592, @jquense)
 - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04)
 - update deps
 - [meta] ensure a license and readme is present in all packages when published
ljharb added a commit that referenced this pull request Aug 25, 2018
 - [New] Add forwardRef support (#1592, @jquense)
 - [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke)
 - [New] Add pointer events support (#1753, @ljharb)
 - [New] Add `displayNameOfNode`, `isValidElementType` (#1701, @jquense)
 - [New] pass the adapter into `createMountWrapper` (#1592, @jquense)
 - [Fix] preemptively fix compat with React v16.4.3 (#1790, #1778, @gaearon, @aweary)
 - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04)
 - update deps
 - [meta] ensure a license and readme is present in all packages when published
ljharb added a commit that referenced this pull request Aug 25, 2018
 - [New] Add forwardRef support (#1592, @jquense)
 - [New] Add Portal support (#1760, #1761, #1772, #1774, @jgzuke)
 - [New] Add pointer events support (#1753, @ljharb)
 - [Fix] preemptively fix compat with React v16.4.3 (#1790, #1778, @gaearon, @aweary)
 - [Fix] `shallow`: prevent rerenders with PureComponents (#1786, @koba04)
 - [Fix] `shallow`: skip updates when nextState is `null` or `undefined` (#1785, @koba04)
 - [Fix] `shallow`: `setState` after `setProps` calls `componentWillReceiveProps` (#1779, @peanutenthusiast)
 - [Fix] `mount`/`shallow`: be stricter on the wrapper’s setState/setProps callback
 - [Fix] `shallow`/`mount`: improve error message when wrapping invalid elements (#1759, @jgzuke)
 - update deps
 - [Refactor] remove most uses of lodash
 - [meta] ensure a license and readme is present in all packages when published
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants