Skip to content

Commit

Permalink
[Fix] selectors: unwrap memo elements - in both directions.
Browse files Browse the repository at this point in the history
Note: although this is an important `memo` bugfix, this *might* be a breaking change (note the `find` test change from 1 to 2) but since it should be exceedingly rare for tests to have a reference to both the memoized and unmemoized form of a component, my hope is that it won’t impact anybody.

Fixes #2146.
  • Loading branch information
ljharb committed Jun 1, 2019
1 parent 7986308 commit ddb0627
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 10 deletions.
13 changes: 7 additions & 6 deletions packages/enzyme-adapter-react-16/src/ReactSixteenAdapter.js
Expand Up @@ -108,6 +108,10 @@ function nodeTypeFromType(type) {
return utilNodeTypeFromType(type);
}

function unmemoType(type) {
return isMemo(type) ? type.type : type;
}

function elementToTree(el) {
if (!isPortal(el)) {
return utilElementToTree(el, elementToTree);
Expand Down Expand Up @@ -764,19 +768,16 @@ class ReactSixteenAdapter extends EnzymeAdapter {
nodeToElement(node) {
if (!node || typeof node !== 'object') return null;
const { type } = node;
return React.createElement(isMemo(type) ? type.type : type, propsWithKeysAndRef(node));
return React.createElement(unmemoType(type), propsWithKeysAndRef(node));
}

// eslint-disable-next-line class-methods-use-this
matchesElementType(node, matchingType) {
if (!node) {
return node;
}
const { type } = node;

const nodeType = isMemo(type) ? type.type : type;
const matchingTypeType = isMemo(matchingType) ? matchingType.type : matchingType;
// console.log('**', isMemo(type), type === matchingType, type.type === matchingType, type === matchingType.type, type.type === matchingType.type);
return nodeType === matchingTypeType;
return unmemoType(type) === unmemoType(matchingType);
}

elementToNode(element) {
Expand Down
8 changes: 5 additions & 3 deletions packages/enzyme-test-suite/test/shared/methods/find.jsx
Expand Up @@ -909,11 +909,13 @@ export default function describeFind({
</div>
</Memo(InnerFoo)>`;
expect(wrapper.debug()).to.equal(expectedDebug);
expect(wrapper.find('InnerComp')).to.have.lengthOf(1);
expect(wrapper.find('Memo(InnerComp)')).to.have.lengthOf(1);
const inner = wrapper.find('InnerComp');
expect(inner).to.have.lengthOf(1);
const memoInner = wrapper.find('Memo(InnerComp)');
expect(memoInner).to.have.lengthOf(1);
expect(wrapper.find('.bar')).to.have.lengthOf(1);
expect(wrapper.find('.qoo').text()).to.equal('qux');
expect(wrapper.find(InnerMemo)).to.have.lengthOf(1);
expect(wrapper.find(InnerMemo)).to.have.lengthOf(inner.length + memoInner.length); // 2
});

it('works with a class component', () => {
Expand Down
52 changes: 52 additions & 0 deletions packages/enzyme-test-suite/test/shared/methods/is.jsx
@@ -1,8 +1,19 @@
import React from 'react';
import { expect } from 'chai';

import {
describeIf,
} from '../../_helpers';
import { is } from '../../_helpers/version';

import {
memo,
forwardRef,
} from '../../_helpers/react-compat';

export default function describeIs({
Wrap,
WrapRendered,
}) {
describe('.is(selector)', () => {
it('returns true when selector matches current element', () => {
Expand Down Expand Up @@ -36,5 +47,46 @@ export default function describeIs({
const wrapper = Wrap(<div className="bar baz" />);
expect(wrapper.is('.foo')).to.equal(false);
});

class RendersDiv extends React.Component {
render() {
return <div />;
}
}
const Memoized = memo && memo(RendersDiv);
const ForwardRef = forwardRef && forwardRef(() => <RendersDiv />);
const MemoForwardRef = memo && memo(() => <ForwardRef />);

class RendersChildren extends React.Component {
render() {
const { children } = this.props;
return children;
}
}

it('recognizes nonmemoized', () => {
const wrapper = WrapRendered(<RendersChildren><RendersDiv /></RendersChildren>);
expect(wrapper.is(RendersDiv)).to.equal(true);
});

describeIf(is('>= 16.3'), 'forwardRef', () => {
it('recognizes forwardRef', () => {
const wrapper = WrapRendered(<RendersChildren><ForwardRef /></RendersChildren>);
expect(wrapper.is(ForwardRef)).to.equal(true);
});
});

describeIf(is('>= 16.6'), 'React.memo', () => {
it('recognizes memoized and inner', () => {
const wrapper = WrapRendered(<RendersChildren><Memoized /></RendersChildren>);
expect(wrapper.is(Memoized)).to.equal(true);
// expect(wrapper.is(RendersDiv)).to.equal(true);
});

it('recognizes memoized forwardRef and inner', () => {
const wrapper = WrapRendered(<RendersChildren><MemoForwardRef /></RendersChildren>);
expect(wrapper.is(MemoForwardRef)).to.equal(true);
});
});
});
}
1 change: 1 addition & 0 deletions packages/enzyme/src/EnzymeAdapter.js
Expand Up @@ -22,6 +22,7 @@ class EnzymeAdapter {
throw unimplementedError('nodeToElement', 'EnzymeAdapter');
}

// eslint-disable-next-line class-methods-use-this
matchesElementType(node, matchingType) {
if (!node) {
return node;
Expand Down
2 changes: 1 addition & 1 deletion packages/enzyme/src/selectors.js
Expand Up @@ -274,7 +274,7 @@ export function buildPredicate(selector) {
? adapter.isValidElementType(selector)
: typeof selector === 'function';
if (isElementType) {
return node => node && node.type === selector;
return node => adapter.matchesElementType(node, selector);
}
// If the selector is an non-empty object, treat the keys/values as props
if (typeof selector === 'object') {
Expand Down

0 comments on commit ddb0627

Please sign in to comment.