Skip to content

Commit

Permalink
Revert "React.pure automatically forwards ref"
Browse files Browse the repository at this point in the history
Reverts #13822. We're not sure we want to do this.
  • Loading branch information
sophiebits committed Oct 19, 2018
1 parent 4dd772a commit d1219e2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 66 deletions.
7 changes: 3 additions & 4 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ function updatePureComponent(
renderExpirationTime: ExpirationTime,
) {
const render = Component.render;
const ref = workInProgress.ref;

if (
current !== null &&
Expand All @@ -253,7 +252,7 @@ function updatePureComponent(
// Default to shallow comparison
let compare = Component.compare;
compare = compare !== null ? compare : shallowEqual;
if (workInProgress.ref === current.ref && compare(prevProps, nextProps)) {
if (compare(prevProps, nextProps)) {
return bailoutOnAlreadyFinishedWork(
current,
workInProgress,
Expand All @@ -268,10 +267,10 @@ function updatePureComponent(
if (__DEV__) {
ReactCurrentOwner.current = workInProgress;
ReactCurrentFiber.setCurrentPhase('render');
nextChildren = render(nextProps, ref);
nextChildren = render(nextProps);
ReactCurrentFiber.setCurrentPhase(null);
} else {
nextChildren = render(nextProps, ref);
nextChildren = render(nextProps);
}

// React DevTools reads this flag.
Expand Down
65 changes: 3 additions & 62 deletions packages/react-reconciler/src/__tests__/ReactPure-test.internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ describe('pure', () => {
// a lazy function component.
sharedTests('normal', (...args) => {
const Pure = React.pure(...args);
function Indirection(props, ref) {
return <Pure {...props} ref={ref} />;
function Indirection(props) {
return <Pure {...props} />;
}
return Promise.resolve(React.forwardRef(Indirection));
return Promise.resolve(Indirection);
});
sharedTests('lazy', (...args) => Promise.resolve(React.pure(...args)));

Expand Down Expand Up @@ -195,65 +195,6 @@ describe('pure', () => {
{withoutStack: true},
);
});

it('forwards ref', async () => {
const {unstable_Suspense: Suspense} = React;
const Transparent = pure((props, ref) => {
return <div ref={ref} />;
});
const divRef = React.createRef();

ReactNoop.render(
<Suspense fallback={<Text text="Loading..." />}>
<Transparent ref={divRef} />
</Suspense>,
);
ReactNoop.flush();
await Promise.resolve();
ReactNoop.flush();
expect(divRef.current.type).toBe('div');
});

it('updates if only ref changes', async () => {
const {unstable_Suspense: Suspense} = React;
const Transparent = pure((props, ref) => {
return [<Text key="text" text="Text" />, <div key="div" ref={ref} />];
});

const divRef = React.createRef();
const divRef2 = React.createRef();

ReactNoop.render(
<Suspense fallback={<Text text="Loading..." />}>
<Transparent ref={divRef} />
</Suspense>,
);
expect(ReactNoop.flush()).toEqual(['Loading...']);
await Promise.resolve();
expect(ReactNoop.flush()).toEqual(['Text']);
expect(divRef.current.type).toBe('div');
expect(divRef2.current).toBe(null);

// Should re-render (new ref)
ReactNoop.render(
<Suspense>
<Transparent ref={divRef2} />
</Suspense>,
);
expect(ReactNoop.flush()).toEqual(['Text']);
expect(divRef.current).toBe(null);
expect(divRef2.current.type).toBe('div');

// Should not re-render (same ref)
ReactNoop.render(
<Suspense>
<Transparent ref={divRef2} />
</Suspense>,
);
expect(ReactNoop.flush()).toEqual([]);
expect(divRef.current).toBe(null);
expect(divRef2.current.type).toBe('div');
});
});
}
});

0 comments on commit d1219e2

Please sign in to comment.