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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Support React.memo in ReactShallowRenderer #14816

Merged
merged 6 commits into from Mar 15, 2019
Merged

Support React.memo in ReactShallowRenderer #14816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
786262d
Support React.memo in ReactShallowRenderer
Feb 11, 2019
f06a74e
Allow Rect.memo to prevent re-renders
Feb 11, 2019
75dcd59
Support memo(forwardRef())
Feb 11, 2019
0dbe1fd
Dont call memo comparison function on initial render
Feb 11, 2019
2230fab
Fix test
gaearon Mar 15, 2019
137f390
Small tweaks
gaearon Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
125 changes: 86 additions & 39 deletions packages/react-test-renderer/src/ReactShallowRenderer.js
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@
*/

import React from 'react';
import {isForwardRef} from 'react-is';
import {isForwardRef, isMemo, ForwardRef} from 'react-is';
import describeComponentFrame from 'shared/describeComponentFrame';
import getComponentName from 'shared/getComponentName';
import shallowEqual from 'shared/shallowEqual';
Expand Down Expand Up @@ -500,7 +500,8 @@ class ReactShallowRenderer {
element.type,
);
invariant(
isForwardRef(element) || typeof element.type === 'function',
isForwardRef(element) ||
(typeof element.type === 'function' || isMemo(element.type)),
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason not to use isValidElementType from react-is here, and asserting the type isn鈥檛 string, rather than repeating some of the logic?

Copy link
Contributor Author

@aweary aweary Feb 12, 2019 •
edited

Choose a reason for hiding this comment

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

Probably because isValidElementType was introduced after this version of the shallow renderer. That's a good point though, because looking at isValidElementType catches more uncommon cases that React technically supports and that fail with the shallow renderer.

For example, you can do:

const MemoFragment = React.memo(React.Fragment)

But rendering that with the shallow render will trigger this invariant. The same goes for other elements that have their own type like React.Suspense, React.ConcurrentMode, React.StrictMode, etc.

'ReactShallowRenderer render(): Shallow rendering works only with custom ' +
'components, but the provided element type was `%s`.',
Array.isArray(element.type)
Expand All @@ -514,22 +515,36 @@ class ReactShallowRenderer {
return;
}

const elementType = isMemo(element.type) ? element.type.type : element.type;
const previousElement = this._element;

this._rendering = true;
this._element = element;
this._context = getMaskedContext(element.type.contextTypes, context);
this._context = getMaskedContext(elementType.contextTypes, context);

// Inner memo component props aren't currently validated in createElement.
if (isMemo(element.type) && elementType.propTypes) {
currentlyValidatingElement = element;
checkPropTypes(
elementType.propTypes,
element.props,
'prop',
getComponentName(elementType),
getStackAddendum,
);
}

if (this._instance) {
this._updateClassComponent(element, this._context);
this._updateClassComponent(elementType, element, this._context);
} else {
if (shouldConstruct(element.type)) {
this._instance = new element.type(
if (shouldConstruct(elementType)) {
this._instance = new elementType(
element.props,
this._context,
this._updater,
);

if (typeof element.type.getDerivedStateFromProps === 'function') {
const partialState = element.type.getDerivedStateFromProps.call(
if (typeof elementType.getDerivedStateFromProps === 'function') {
const partialState = elementType.getDerivedStateFromProps.call(
null,
element.props,
this._instance.state,
Expand All @@ -543,39 +558,59 @@ class ReactShallowRenderer {
}
}

if (element.type.hasOwnProperty('contextTypes')) {
if (elementType.contextTypes) {
currentlyValidatingElement = element;

checkPropTypes(
element.type.contextTypes,
elementType.contextTypes,
this._context,
'context',
getName(element.type, this._instance),
getName(elementType, this._instance),
getStackAddendum,
);

currentlyValidatingElement = null;
}

this._mountClassComponent(element, this._context);
this._mountClassComponent(elementType, element, this._context);
} else {
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = this._dispatcher;
this._prepareToUseHooks(element.type);
try {
if (isForwardRef(element)) {
this._rendered = element.type.render(element.props, element.ref);
} else {
this._rendered = element.type.call(
undefined,
element.props,
this._context,
);
let shouldRender = true;
if (
isMemo(element.type) &&
elementType === this._previousComponentIdentity &&
previousElement !== null
) {
// This is a Memo component that is being re-rendered.
const compare = element.type.compare || shallowEqual;
if (compare(previousElement.props, element.props)) {
shouldRender = false;
}
}
if (shouldRender) {
const prevDispatcher = ReactCurrentDispatcher.current;
ReactCurrentDispatcher.current = this._dispatcher;
this._prepareToUseHooks(elementType);
try {
// elementType could still be a ForwardRef if it was
// nested inside Memo.
if (elementType.$$typeof === ForwardRef) {
invariant(
typeof elementType.render === 'function',
'forwardRef requires a render function but was given %s.',
typeof elementType.render,
);
this._rendered = elementType.render.call(
undefined,
element.props,
element.ref,
);
} else {
this._rendered = elementType(element.props, this._context);
}
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
}
} finally {
ReactCurrentDispatcher.current = prevDispatcher;
this._finishHooks(element, context);
}
this._finishHooks(element, context);
}
}

Expand All @@ -601,7 +636,11 @@ class ReactShallowRenderer {
this._instance = null;
}

_mountClassComponent(element: ReactElement, context: null | Object) {
_mountClassComponent(
elementType: Function,
element: ReactElement,
context: null | Object,
) {
this._instance.context = context;
this._instance.props = element.props;
this._instance.state = this._instance.state || null;
Expand All @@ -616,7 +655,7 @@ class ReactShallowRenderer {
// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof element.type.getDerivedStateFromProps !== 'function' &&
typeof elementType.getDerivedStateFromProps !== 'function' &&
typeof this._instance.getSnapshotBeforeUpdate !== 'function'
) {
if (typeof this._instance.componentWillMount === 'function') {
Expand All @@ -638,8 +677,12 @@ class ReactShallowRenderer {
// because DOM refs are not available.
}

_updateClassComponent(element: ReactElement, context: null | Object) {
const {props, type} = element;
_updateClassComponent(
elementType: Function,
element: ReactElement,
context: null | Object,
) {
const {props} = element;

const oldState = this._instance.state || emptyObject;
const oldProps = this._instance.props;
Expand All @@ -648,7 +691,7 @@ class ReactShallowRenderer {
// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof element.type.getDerivedStateFromProps !== 'function' &&
typeof elementType.getDerivedStateFromProps !== 'function' &&
typeof this._instance.getSnapshotBeforeUpdate !== 'function'
) {
if (typeof this._instance.componentWillReceiveProps === 'function') {
Expand All @@ -664,8 +707,8 @@ class ReactShallowRenderer {

// Read state after cWRP in case it calls setState
let state = this._newState || oldState;
if (typeof type.getDerivedStateFromProps === 'function') {
const partialState = type.getDerivedStateFromProps.call(
if (typeof elementType.getDerivedStateFromProps === 'function') {
const partialState = elementType.getDerivedStateFromProps.call(
null,
props,
state,
Expand All @@ -685,7 +728,10 @@ class ReactShallowRenderer {
state,
context,
);
} else if (type.prototype && type.prototype.isPureReactComponent) {
} else if (
elementType.prototype &&
elementType.prototype.isPureReactComponent
) {
shouldUpdate =
!shallowEqual(oldProps, props) || !shallowEqual(oldState, state);
}
Expand All @@ -694,7 +740,7 @@ class ReactShallowRenderer {
// In order to support react-lifecycles-compat polyfilled components,
// Unsafe lifecycles should not be invoked for components using the new APIs.
if (
typeof element.type.getDerivedStateFromProps !== 'function' &&
typeof elementType.getDerivedStateFromProps !== 'function' &&
typeof this._instance.getSnapshotBeforeUpdate !== 'function'
) {
if (typeof this._instance.componentWillUpdate === 'function') {
Expand Down Expand Up @@ -729,7 +775,8 @@ function getDisplayName(element) {
} else if (typeof element.type === 'string') {
return element.type;
} else {
return element.type.displayName || element.type.name || 'Unknown';
const elementType = isMemo(element.type) ? element.type.type : element.type;
return elementType.displayName || elementType.name || 'Unknown';
}
}

Expand Down
111 changes: 111 additions & 0 deletions packages/react-test-renderer/src/__tests__/ReactShallowRenderer-test.js
View file
Open in desktop
Expand Up @@ -1454,4 +1454,115 @@ describe('ReactShallowRenderer', () => {
shallowRenderer.render(<Foo foo="bar" />);
expect(logs).toEqual([undefined]);
});

it('should handle memo', () => {
function Foo() {
return <div>foo</div>;
}
const MemoFoo = React.memo(Foo);
const shallowRenderer = createRenderer();
shallowRenderer.render(<MemoFoo />);
});

it('should enable React.memo to prevent a re-render', () => {
const logs = [];
const Foo = React.memo(({count}) => {
logs.push(`Foo: ${count}`);
return <div>{count}</div>;
});
const Bar = React.memo(({count}) => {
logs.push(`Bar: ${count}`);
return <div>{count}</div>;
});
const shallowRenderer = createRenderer();
shallowRenderer.render(<Foo count={1} />);
expect(logs).toEqual(['Foo: 1']);
logs.length = 0;
// Rendering the same element with the same props should be prevented
shallowRenderer.render(<Foo count={1} />);
expect(logs).toEqual([]);
// A different element with the same props should cause a re-render
shallowRenderer.render(<Bar count={1} />);
expect(logs).toEqual(['Bar: 1']);
});

it('should respect a custom comparison function with React.memo', () => {
let renderCount = 0;
function areEqual(props, nextProps) {
return props.foo === nextProps.foo;
}
const Foo = React.memo(({foo, bar}) => {
renderCount++;
return (
<div>
{foo} {bar}
</div>
);
}, areEqual);

const shallowRenderer = createRenderer();
shallowRenderer.render(<Foo foo={1} bar={1} />);
expect(renderCount).toBe(1);
// Change a prop that the comparison funciton ignores
shallowRenderer.render(<Foo foo={1} bar={2} />);
expect(renderCount).toBe(1);
shallowRenderer.render(<Foo foo={2} bar={2} />);
expect(renderCount).toBe(2);
});

it('should not call the comparison function with React.memo on the initial render', () => {
const areEqual = jest.fn(() => false);
const SomeComponent = React.memo(({foo}) => {
return <div>{foo}</div>;
}, areEqual);
const shallowRenderer = createRenderer();
shallowRenderer.render(<SomeComponent foo={1} />);
expect(areEqual).not.toHaveBeenCalled();
expect(shallowRenderer.getRenderOutput()).toEqual(<div>{1}</div>);
});

it('should handle memo(forwardRef())', () => {
const testRef = React.createRef();
const SomeComponent = React.forwardRef((props, ref) => {
expect(ref).toEqual(testRef);
return (
<div>
<span className="child1" />
<span className="child2" />
</div>
);
});

const SomeMemoComponent = React.memo(SomeComponent);

const shallowRenderer = createRenderer();
const result = shallowRenderer.render(<SomeMemoComponent ref={testRef} />);

expect(result.type).toBe('div');
expect(result.props.children).toEqual([
<span className="child1" />,
<span className="child2" />,
]);
});

it('should warn for forwardRef(memo())', () => {
const testRef = React.createRef();
const SomeMemoComponent = React.memo(({foo}) => {
return <div>{foo}</div>;
});
const shallowRenderer = createRenderer();
expect(() => {
expect(() => {
const SomeComponent = React.forwardRef(SomeMemoComponent);
shallowRenderer.render(<SomeComponent ref={testRef} />);
}).toWarnDev(
'Warning: forwardRef requires a render function but received ' +
'a `memo` component. Instead of forwardRef(memo(...)), use ' +
'memo(forwardRef(...))',
{withoutStack: true},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is failing saying Expected test not to call console.error(), but I'm calling toWarnDev here so I don't think that's right?

);
}).toThrowError(
'forwardRef requires a render function but was given object.',
);
});
});