Skip to content

Commit c19335b

Browse files
committed
Allow forwardRef to be composed with pure
This uses a different approach than the moddable approach because that approach also started getting quite complex and affects tags and symbols. This way we don't affect the perf of pure and when we drop forwardRef we can just drop this path. This also handles the composition of multiple pure wrappers with custom comparisons.
1 parent fa65c58 commit c19335b

File tree

4 files changed

+181
-3
lines changed

4 files changed

+181
-3
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ function updateForwardRef(
196196
workInProgress: Fiber,
197197
type: any,
198198
nextProps: any,
199+
updateExpirationTime: ExpirationTime,
199200
renderExpirationTime: ExpirationTime,
200201
) {
201202
const render = type.render;
@@ -212,6 +213,23 @@ function updateForwardRef(
212213
renderExpirationTime,
213214
);
214215
}
216+
} else if (
217+
current !== null &&
218+
type.compare !== undefined &&
219+
(updateExpirationTime === NoWork ||
220+
updateExpirationTime > renderExpirationTime)
221+
) {
222+
const prevProps = current.memoizedProps;
223+
// Default to shallow comparison
224+
let compare = type.compare;
225+
compare = compare !== null ? compare : shallowEqual;
226+
if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) {
227+
return bailoutOnAlreadyFinishedWork(
228+
current,
229+
workInProgress,
230+
renderExpirationTime,
231+
);
232+
}
215233
}
216234

217235
let nextChildren;
@@ -769,6 +787,7 @@ function mountIndeterminateComponent(
769787
workInProgress,
770788
Component,
771789
resolvedProps,
790+
updateExpirationTime,
772791
renderExpirationTime,
773792
);
774793
break;
@@ -1561,6 +1580,7 @@ function beginWork(
15611580
workInProgress,
15621581
type,
15631582
workInProgress.pendingProps,
1583+
updateExpirationTime,
15641584
renderExpirationTime,
15651585
);
15661586
}
@@ -1573,6 +1593,7 @@ function beginWork(
15731593
workInProgress,
15741594
Component,
15751595
resolveDefaultProps(Component, unresolvedProps),
1596+
updateExpirationTime,
15761597
renderExpirationTime,
15771598
);
15781599
workInProgress.memoizedProps = unresolvedProps;

packages/react/src/__tests__/forwardRef-test.js

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,4 +226,131 @@ describe('forwardRef', () => {
226226
' in Foo (at **)',
227227
);
228228
});
229+
230+
it('should not bailout if forwardRef is not wrapped in pure', () => {
231+
const Component = props => <div {...props} />;
232+
233+
let renderCount = 0;
234+
235+
const RefForwardingComponent = React.forwardRef((props, ref) => {
236+
renderCount++;
237+
return <Component {...props} forwardedRef={ref} />;
238+
});
239+
240+
const ref = React.createRef();
241+
242+
ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
243+
ReactNoop.flush();
244+
expect(renderCount).toBe(1);
245+
246+
ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
247+
ReactNoop.flush();
248+
expect(renderCount).toBe(2);
249+
});
250+
251+
it('should bailout if forwardRef is wrapped in pure', () => {
252+
const Component = props => <div ref={props.forwardedRef} />;
253+
254+
let renderCount = 0;
255+
256+
const RefForwardingComponent = React.pure(
257+
React.forwardRef((props, ref) => {
258+
renderCount++;
259+
return <Component {...props} forwardedRef={ref} />;
260+
}),
261+
);
262+
263+
const ref = React.createRef();
264+
265+
ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
266+
ReactNoop.flush();
267+
expect(renderCount).toBe(1);
268+
269+
expect(ref.current.type).toBe('div');
270+
271+
ReactNoop.render(<RefForwardingComponent ref={ref} optional="foo" />);
272+
ReactNoop.flush();
273+
expect(renderCount).toBe(1);
274+
275+
const differentRef = React.createRef();
276+
277+
ReactNoop.render(
278+
<RefForwardingComponent ref={differentRef} optional="foo" />,
279+
);
280+
ReactNoop.flush();
281+
expect(renderCount).toBe(2);
282+
283+
expect(ref.current).toBe(null);
284+
expect(differentRef.current.type).toBe('div');
285+
286+
ReactNoop.render(<RefForwardingComponent ref={ref} optional="bar" />);
287+
ReactNoop.flush();
288+
expect(renderCount).toBe(3);
289+
});
290+
291+
it('should custom pure comparisons to compose', () => {
292+
const Component = props => <div ref={props.forwardedRef} />;
293+
294+
let renderCount = 0;
295+
296+
const RefForwardingComponent = React.pure(
297+
React.forwardRef((props, ref) => {
298+
renderCount++;
299+
return <Component {...props} forwardedRef={ref} />;
300+
}),
301+
(o, p) => o.a === p.a && o.b === p.b,
302+
);
303+
304+
const ref = React.createRef();
305+
306+
ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="0" c="1" />);
307+
ReactNoop.flush();
308+
expect(renderCount).toBe(1);
309+
310+
expect(ref.current.type).toBe('div');
311+
312+
// Changing either a or b rerenders
313+
ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="1" c="1" />);
314+
ReactNoop.flush();
315+
expect(renderCount).toBe(2);
316+
317+
// Changing c doesn't rerender
318+
ReactNoop.render(<RefForwardingComponent ref={ref} a="0" b="1" c="2" />);
319+
ReactNoop.flush();
320+
expect(renderCount).toBe(2);
321+
322+
const ComposedPure = React.pure(
323+
RefForwardingComponent,
324+
(o, p) => o.a === p.a && o.c === p.c,
325+
);
326+
327+
ReactNoop.render(<ComposedPure ref={ref} a="0" b="0" c="0" />);
328+
ReactNoop.flush();
329+
expect(renderCount).toBe(3);
330+
331+
// Changing just b no longer updates
332+
ReactNoop.render(<ComposedPure ref={ref} a="0" b="1" c="0" />);
333+
ReactNoop.flush();
334+
expect(renderCount).toBe(3);
335+
336+
// Changing just a and c updates
337+
ReactNoop.render(<ComposedPure ref={ref} a="2" b="2" c="2" />);
338+
ReactNoop.flush();
339+
expect(renderCount).toBe(4);
340+
341+
// Changing just c does not update
342+
ReactNoop.render(<ComposedPure ref={ref} a="2" b="2" c="3" />);
343+
ReactNoop.flush();
344+
expect(renderCount).toBe(4);
345+
346+
// Changing ref still rerenders
347+
const differentRef = React.createRef();
348+
349+
ReactNoop.render(<ComposedPure ref={differentRef} a="2" b="2" c="3" />);
350+
ReactNoop.flush();
351+
expect(renderCount).toBe(5);
352+
353+
expect(ref.current).toBe(null);
354+
expect(differentRef.current.type).toBe('div');
355+
});
229356
});

packages/react/src/forwardRef.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,15 @@ import {REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols';
99

1010
import warningWithoutStack from 'shared/warningWithoutStack';
1111

12+
export type ForwardRef<Props> = {
13+
$$typeof: Symbol,
14+
render: (props: Props, ref: React$Ref<ElementType>) => React$Node,
15+
compare: void | null | ((oldProps: Props, newProps: Props) => boolean),
16+
};
17+
1218
export default function forwardRef<Props, ElementType: React$ElementType>(
1319
render: (props: Props, ref: React$Ref<ElementType>) => React$Node,
14-
) {
20+
): ForwardRef<Props> {
1521
if (__DEV__) {
1622
if (typeof render !== 'function') {
1723
warningWithoutStack(
@@ -42,5 +48,6 @@ export default function forwardRef<Props, ElementType: React$ElementType>(
4248
return {
4349
$$typeof: REACT_FORWARD_REF_TYPE,
4450
render,
51+
compare: undefined,
4552
};
4653
}

packages/react/src/pure.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,37 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import {REACT_PURE_TYPE} from 'shared/ReactSymbols';
8+
import type {ForwardRef} from './forwardRef';
9+
10+
import {REACT_PURE_TYPE, REACT_FORWARD_REF_TYPE} from 'shared/ReactSymbols';
911

1012
import warningWithoutStack from 'shared/warningWithoutStack';
1113

14+
function composeComparisonFunctions(inner, outer) {
15+
return function(oldProps, newProps) {
16+
// Either is allowed to block the update.
17+
return outer(oldProps, newProps) || inner(oldProps, newProps);
18+
};
19+
}
20+
1221
export default function pure<Props>(
13-
render: (props: Props) => React$Node,
22+
render: (props: Props) => React$Node | ForwardRef,
1423
compare?: (oldProps: Props, newProps: Props) => boolean,
1524
) {
25+
if (
26+
typeof render === 'object' &&
27+
render.$$typeof === REACT_FORWARD_REF_TYPE
28+
) {
29+
let forwardRef = (render: ForwardRef);
30+
if (forwardRef.compare !== undefined && forwardRef.compare !== null) {
31+
compare = composeComparisonFunctions(forwardRef.compare, compare);
32+
}
33+
return {
34+
$$typeof: REACT_FORWARD_REF_TYPE,
35+
render: forwardRef.render,
36+
compare: compare === undefined ? null : compare,
37+
};
38+
}
1639
if (__DEV__) {
1740
if (typeof render !== 'function') {
1841
warningWithoutStack(

0 commit comments

Comments
 (0)