From 15b11d23f960c158a7e99679bf62041ce16aed7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 20 Oct 2018 00:32:16 -0700 Subject: [PATCH] Allow arbitrary types to be wrapped in pure (#13903) * Allow arbitrary types to be wrapped in pure This creates an outer fiber that container the pure check and an inner fiber that represents which ever type of component. * Add optimized fast path for simple pure function components Special cased when there are no defaultProps and it's a simple function component instead of class. This doesn't require an extra fiber. We could make it so that this also works with custom comparer but that means we have to go through one extra indirection to get to it. Maybe it's worth it, donno. --- packages/react-reconciler/src/ReactFiber.js | 42 +++--- .../src/ReactFiberBeginWork.js | 124 ++++++++++++----- .../src/ReactFiberCompleteWork.js | 2 + .../src/__tests__/ReactPure-test.internal.js | 44 ++++-- ...ReactIncrementalPerf-test.internal.js.snap | 2 +- .../src/ReactTestRenderer.js | 2 + .../react/src/__tests__/forwardRef-test.js | 127 ++++++++++++++++++ packages/react/src/pure.js | 20 +-- packages/shared/ReactWorkTags.js | 3 +- packages/shared/getComponentName.js | 2 +- 10 files changed, 294 insertions(+), 74 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.js b/packages/react-reconciler/src/ReactFiber.js index 78f25688009d3..ff251cbf68c76 100644 --- a/packages/react-reconciler/src/ReactFiber.js +++ b/packages/react-reconciler/src/ReactFiber.js @@ -299,10 +299,15 @@ function shouldConstruct(Component: Function) { return !!(prototype && prototype.isReactComponent); } -export function resolveLazyComponentTag( - fiber: Fiber, - Component: Function, -): WorkTag { +export function isSimpleFunctionComponent(type: any) { + return ( + typeof type === 'function' && + !shouldConstruct(type) && + type.defaultProps === undefined + ); +} + +export function resolveLazyComponentTag(Component: Function): WorkTag { if (typeof Component === 'function') { return shouldConstruct(Component) ? ClassComponent : FunctionComponent; } else if (Component !== undefined && Component !== null) { @@ -406,20 +411,15 @@ export function createHostRootFiber(isConcurrent: boolean): Fiber { return createFiber(HostRoot, null, null, mode); } -function createFiberFromElementWithoutDebugInfo( - element: ReactElement, +export function createFiberFromTypeAndProps( + type: any, // React$ElementType + key: null | string, + pendingProps: any, + owner: null | Fiber, mode: TypeOfMode, expirationTime: ExpirationTime, ): Fiber { - let owner = null; - if (__DEV__) { - owner = element._owner; - } - let fiber; - const type = element.type; - const key = element.key; - const pendingProps = element.props; let fiberTag = IndeterminateComponent; // The resolved type is set if we know what the final type will be. I.e. it's not lazy. @@ -522,8 +522,18 @@ export function createFiberFromElement( mode: TypeOfMode, expirationTime: ExpirationTime, ): Fiber { - const fiber = createFiberFromElementWithoutDebugInfo( - element, + let owner = null; + if (__DEV__) { + owner = element._owner; + } + const type = element.type; + const key = element.key; + const pendingProps = element.props; + const fiber = createFiberFromTypeAndProps( + type, + key, + pendingProps, + owner, mode, expirationTime, ); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index e9304857e4abd..cd65c864edad9 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -31,6 +31,7 @@ import { Profiler, SuspenseComponent, PureComponent, + SimplePureComponent, LazyComponent, } from 'shared/ReactWorkTags'; import { @@ -103,8 +104,10 @@ import { import {readLazyComponentType} from './ReactFiberLazyComponent'; import { resolveLazyComponentTag, + createFiberFromTypeAndProps, createFiberFromFragment, createWorkInProgress, + isSimpleFunctionComponent, } from './ReactFiber'; const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner; @@ -235,19 +238,49 @@ function updatePureComponent( nextProps: any, updateExpirationTime, renderExpirationTime: ExpirationTime, -) { - const render = Component.render; - +): null | Fiber { + if (current === null) { + let type = Component.type; + if (isSimpleFunctionComponent(type) && Component.compare === null) { + // If this is a plain function component without default props, + // and with only the default shallow comparison, we upgrade it + // to a SimplePureComponent to allow fast path updates. + workInProgress.tag = SimplePureComponent; + workInProgress.type = type; + return updateSimplePureComponent( + current, + workInProgress, + type, + nextProps, + updateExpirationTime, + renderExpirationTime, + ); + } + let child = createFiberFromTypeAndProps( + Component.type, + null, + nextProps, + null, + workInProgress.mode, + renderExpirationTime, + ); + child.ref = workInProgress.ref; + child.return = workInProgress; + workInProgress.child = child; + return child; + } + let currentChild = ((current.child: any): Fiber); // This is always exactly one child if ( - current !== null && - (updateExpirationTime === NoWork || - updateExpirationTime > renderExpirationTime) + updateExpirationTime === NoWork || + updateExpirationTime > renderExpirationTime ) { - const prevProps = current.memoizedProps; + // This will be the props with resolved defaultProps, + // unlike current.memoizedProps which will be the unresolved ones. + const prevProps = currentChild.memoizedProps; // Default to shallow comparison let compare = Component.compare; compare = compare !== null ? compare : shallowEqual; - if (compare(prevProps, nextProps)) { + if (compare(prevProps, nextProps) && current.ref === workInProgress.ref) { return bailoutOnAlreadyFinishedWork( current, workInProgress, @@ -255,28 +288,49 @@ function updatePureComponent( ); } } + let newChild = createWorkInProgress( + currentChild, + nextProps, + renderExpirationTime, + ); + newChild.ref = workInProgress.ref; + newChild.return = workInProgress; + workInProgress.child = newChild; + return newChild; +} - // The rest is a fork of updateFunctionComponent - let nextChildren; - prepareToReadContext(workInProgress, renderExpirationTime); - if (__DEV__) { - ReactCurrentOwner.current = workInProgress; - ReactCurrentFiber.setCurrentPhase('render'); - nextChildren = render(nextProps); - ReactCurrentFiber.setCurrentPhase(null); - } else { - nextChildren = render(nextProps); +function updateSimplePureComponent( + current: Fiber | null, + workInProgress: Fiber, + Component: any, + nextProps: any, + updateExpirationTime, + renderExpirationTime: ExpirationTime, +): null | Fiber { + if ( + current !== null && + (updateExpirationTime === NoWork || + updateExpirationTime > renderExpirationTime) + ) { + const prevProps = current.memoizedProps; + if ( + shallowEqual(prevProps, nextProps) && + current.ref === workInProgress.ref + ) { + return bailoutOnAlreadyFinishedWork( + current, + workInProgress, + renderExpirationTime, + ); + } } - - // React DevTools reads this flag. - workInProgress.effectTag |= PerformedWork; - reconcileChildren( + return updateFunctionComponent( current, workInProgress, - nextChildren, + Component, + nextProps, renderExpirationTime, ); - return workInProgress.child; } function updateFragment( @@ -725,10 +779,7 @@ function mountLazyComponent( let Component = readLazyComponentType(elementType); // Store the unwrapped component in the type. workInProgress.type = Component; - const resolvedTag = (workInProgress.tag = resolveLazyComponentTag( - workInProgress, - Component, - )); + const resolvedTag = (workInProgress.tag = resolveLazyComponentTag(Component)); startWorkTimer(workInProgress); const resolvedProps = resolveDefaultProps(Component, props); let child; @@ -768,7 +819,7 @@ function mountLazyComponent( null, workInProgress, Component, - resolvedProps, + resolveDefaultProps(Component.type, resolvedProps), // The inner type can have defaults too updateExpirationTime, renderExpirationTime, ); @@ -1564,10 +1615,7 @@ function beginWork( case PureComponent: { const type = workInProgress.type; const unresolvedProps = workInProgress.pendingProps; - const resolvedProps = - workInProgress.elementType === type - ? unresolvedProps - : resolveDefaultProps(type, unresolvedProps); + const resolvedProps = resolveDefaultProps(type.type, unresolvedProps); return updatePureComponent( current, workInProgress, @@ -1577,6 +1625,16 @@ function beginWork( renderExpirationTime, ); } + case SimplePureComponent: { + return updateSimplePureComponent( + current, + workInProgress, + workInProgress.type, + workInProgress.pendingProps, + updateExpirationTime, + renderExpirationTime, + ); + } default: invariant( false, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 2aa17aa1e024a..12069b9f919d7 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -35,6 +35,7 @@ import { Profiler, SuspenseComponent, PureComponent, + SimplePureComponent, LazyComponent, } from 'shared/ReactWorkTags'; import {Placement, Ref, Update} from 'shared/ReactSideEffectTags'; @@ -539,6 +540,7 @@ function completeWork( break; case LazyComponent: break; + case SimplePureComponent: case FunctionComponent: break; case ClassComponent: { diff --git a/packages/react-reconciler/src/__tests__/ReactPure-test.internal.js b/packages/react-reconciler/src/__tests__/ReactPure-test.internal.js index a800483b42c13..081e15cd034cf 100644 --- a/packages/react-reconciler/src/__tests__/ReactPure-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactPure-test.internal.js @@ -183,21 +183,49 @@ describe('pure', () => { expect(ReactNoop.getChildren()).toEqual([span(1)]); }); - it('warns for class components', () => { - class SomeClass extends React.Component { + it('supports non-pure class components', async () => { + const {unstable_Suspense: Suspense} = React; + + class CounterInner extends React.Component { + static defaultProps = {suffix: '!'}; render() { - return null; + return ; } } - expect(() => pure(SomeClass)).toWarnDev( - 'pure: The first argument must be a function component.', - {withoutStack: true}, + const Counter = pure(CounterInner); + + ReactNoop.render( + }> + + , + ); + expect(ReactNoop.flush()).toEqual(['Loading...']); + await Promise.resolve(); + expect(ReactNoop.flush()).toEqual(['0!']); + expect(ReactNoop.getChildren()).toEqual([span('0!')]); + + // Should bail out because props have not changed + ReactNoop.render( + + + , + ); + expect(ReactNoop.flush()).toEqual([]); + expect(ReactNoop.getChildren()).toEqual([span('0!')]); + + // Should update because count prop changed + ReactNoop.render( + + + , ); + expect(ReactNoop.flush()).toEqual(['1!']); + expect(ReactNoop.getChildren()).toEqual([span('1!')]); }); - it('warns if first argument is not a function', () => { + it('warns if first argument is undefined', () => { expect(() => pure()).toWarnDev( - 'pure: The first argument must be a function component. Instead ' + + 'pure: The first argument must be a component. Instead ' + 'received: undefined', {withoutStack: true}, ); diff --git a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap index 24f4454f5e23b..f330c1f2944dd 100644 --- a/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap +++ b/packages/react-reconciler/src/__tests__/__snapshots__/ReactIncrementalPerf-test.internal.js.snap @@ -415,7 +415,7 @@ exports[`ReactDebugFiberPerf supports pure 1`] = ` ⚛ (React Tree Reconciliation: Completed Root) ⚛ Parent [mount] - ⚛ Pure(Foo) [mount] + ⚛ Foo [mount] ⚛ (Committing Changes) ⚛ (Committing Snapshot Effects: 0 Total) diff --git a/packages/react-test-renderer/src/ReactTestRenderer.js b/packages/react-test-renderer/src/ReactTestRenderer.js index fe4398eee692d..00bada86fb111 100644 --- a/packages/react-test-renderer/src/ReactTestRenderer.js +++ b/packages/react-test-renderer/src/ReactTestRenderer.js @@ -28,6 +28,7 @@ import { ForwardRef, Profiler, PureComponent, + SimplePureComponent, } from 'shared/ReactWorkTags'; import invariant from 'shared/invariant'; import ReactVersion from 'shared/ReactVersion'; @@ -166,6 +167,7 @@ function toTree(node: ?Fiber) { rendered: childrenToTree(node.child), }; case FunctionComponent: + case SimplePureComponent: return { nodeType: 'component', type: node.type, diff --git a/packages/react/src/__tests__/forwardRef-test.js b/packages/react/src/__tests__/forwardRef-test.js index c22424c00e186..e7ef1ffeabbb6 100644 --- a/packages/react/src/__tests__/forwardRef-test.js +++ b/packages/react/src/__tests__/forwardRef-test.js @@ -226,4 +226,131 @@ describe('forwardRef', () => { ' in Foo (at **)', ); }); + + it('should not bailout if forwardRef is not wrapped in pure', () => { + const Component = props =>
; + + let renderCount = 0; + + const RefForwardingComponent = React.forwardRef((props, ref) => { + renderCount++; + return ; + }); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(1); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(2); + }); + + it('should bailout if forwardRef is wrapped in pure', () => { + const Component = props =>
; + + let renderCount = 0; + + const RefForwardingComponent = React.pure( + React.forwardRef((props, ref) => { + renderCount++; + return ; + }), + ); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(1); + + expect(ref.current.type).toBe('div'); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(1); + + const differentRef = React.createRef(); + + ReactNoop.render( + , + ); + ReactNoop.flush(); + expect(renderCount).toBe(2); + + expect(ref.current).toBe(null); + expect(differentRef.current.type).toBe('div'); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(3); + }); + + it('should custom pure comparisons to compose', () => { + const Component = props =>
; + + let renderCount = 0; + + const RefForwardingComponent = React.pure( + React.forwardRef((props, ref) => { + renderCount++; + return ; + }), + (o, p) => o.a === p.a && o.b === p.b, + ); + + const ref = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(1); + + expect(ref.current.type).toBe('div'); + + // Changing either a or b rerenders + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(2); + + // Changing c doesn't rerender + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(2); + + const ComposedPure = React.pure( + RefForwardingComponent, + (o, p) => o.a === p.a && o.c === p.c, + ); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(3); + + // Changing just b no longer updates + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(3); + + // Changing just a and c updates + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(4); + + // Changing just c does not update + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(4); + + // Changing ref still rerenders + const differentRef = React.createRef(); + + ReactNoop.render(); + ReactNoop.flush(); + expect(renderCount).toBe(5); + + expect(ref.current).toBe(null); + expect(differentRef.current.type).toBe('div'); + }); }); diff --git a/packages/react/src/pure.js b/packages/react/src/pure.js index 0431c05269ce5..dd981908470a3 100644 --- a/packages/react/src/pure.js +++ b/packages/react/src/pure.js @@ -7,34 +7,26 @@ import {REACT_PURE_TYPE} from 'shared/ReactSymbols'; +import isValidElementType from 'shared/isValidElementType'; import warningWithoutStack from 'shared/warningWithoutStack'; export default function pure( - render: (props: Props) => React$Node, + type: React$ElementType, compare?: (oldProps: Props, newProps: Props) => boolean, ) { if (__DEV__) { - if (typeof render !== 'function') { + if (!isValidElementType(type)) { warningWithoutStack( false, - 'pure: The first argument must be a function component. Instead ' + + 'pure: The first argument must be a component. Instead ' + 'received: %s', - render === null ? 'null' : typeof render, + type === null ? 'null' : typeof type, ); - } else { - const prototype = render.prototype; - if (prototype && prototype.isReactComponent) { - warningWithoutStack( - false, - 'pure: The first argument must be a function component. Classes ' + - 'are not supported. Use React.PureComponent instead.', - ); - } } } return { $$typeof: REACT_PURE_TYPE, - render, + type, compare: compare === undefined ? null : compare, }; } diff --git a/packages/shared/ReactWorkTags.js b/packages/shared/ReactWorkTags.js index be815784f7753..9c03d889aa69f 100644 --- a/packages/shared/ReactWorkTags.js +++ b/packages/shared/ReactWorkTags.js @@ -43,4 +43,5 @@ export const ForwardRef = 11; export const Profiler = 12; export const SuspenseComponent = 13; export const PureComponent = 14; -export const LazyComponent = 15; +export const SimplePureComponent = 15; +export const LazyComponent = 16; diff --git a/packages/shared/getComponentName.js b/packages/shared/getComponentName.js index abc5b04916bca..0bd5c0f71b185 100644 --- a/packages/shared/getComponentName.js +++ b/packages/shared/getComponentName.js @@ -80,7 +80,7 @@ function getComponentName(type: mixed): string | null { case REACT_FORWARD_REF_TYPE: return getWrappedName(type, type.render, 'ForwardRef'); case REACT_PURE_TYPE: - return getWrappedName(type, type.render, 'Pure'); + return getComponentName(type.type); case REACT_LAZY_TYPE: { const thenable: LazyComponent = (type: any); const resolvedThenable = refineResolvedLazyComponent(thenable);