Skip to content

Commit

Permalink
Add ref cleanup function (#25686)
Browse files Browse the repository at this point in the history
Add option for ref function to return a clean up function.

```jsx
<div ref={(_ref) => {
  // Use `_ref`
  return () => {
    // Clean up _ref
  };
}} />
```

If clean up function is not provided. Ref function is called with null
like it has been before.

```jsx
<div ref={(_ref) => {
  if (_ref) {
    // Use _ref
  } else {
    // Clean up _ref
  }
}} />
```
  • Loading branch information
sammy-SC committed Nov 29, 2022
1 parent edbfc63 commit e982254
Show file tree
Hide file tree
Showing 6 changed files with 195 additions and 99 deletions.
162 changes: 133 additions & 29 deletions packages/react-dom/src/__tests__/refs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,35 +401,6 @@ describe('ref swapping', () => {
'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',
);
});

it('should warn about callback refs returning a function', () => {
const container = document.createElement('div');
expect(() => {
ReactDOM.render(<div ref={() => () => {}} />, container);
}).toErrorDev('Unexpected return value from a callback ref in div');

// Cleanup should warn, too.
expect(() => {
ReactDOM.render(<span />, container);
}).toErrorDev('Unexpected return value from a callback ref in div', {
withoutStack: true,
});

// No warning when returning non-functions.
ReactDOM.render(<p ref={() => ({})} />, container);
ReactDOM.render(<p ref={() => null} />, container);
ReactDOM.render(<p ref={() => undefined} />, container);

// Still warns on functions (not deduped).
expect(() => {
ReactDOM.render(<div ref={() => () => {}} />, container);
}).toErrorDev('Unexpected return value from a callback ref in div');
expect(() => {
ReactDOM.unmountComponentAtNode(container);
}).toErrorDev('Unexpected return value from a callback ref in div', {
withoutStack: true,
});
});
});

describe('root level refs', () => {
Expand Down Expand Up @@ -612,3 +583,136 @@ describe('strings refs across renderers', () => {
expect(inst.refs.child2).toBe(div2.firstChild);
});
});

describe('refs return clean up function', () => {
it('calls clean up function if it exists', () => {
const container = document.createElement('div');
let cleanUp = jest.fn();
let setup = jest.fn();

ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
return cleanUp;
}}
/>,
container,
);

ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
}}
/>,
container,
);

expect(setup).toHaveBeenCalledTimes(2);
expect(cleanUp).toHaveBeenCalledTimes(1);
expect(cleanUp.mock.calls[0][0]).toBe(undefined);

ReactDOM.render(<div ref={_ref => {}} />, container);

expect(cleanUp).toHaveBeenCalledTimes(1);
expect(setup).toHaveBeenCalledTimes(3);
expect(setup.mock.calls[2][0]).toBe(null);

cleanUp = jest.fn();
setup = jest.fn();

ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
return cleanUp;
}}
/>,
container,
);

expect(setup).toHaveBeenCalledTimes(1);
expect(cleanUp).toHaveBeenCalledTimes(0);

ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
return cleanUp;
}}
/>,
container,
);

expect(setup).toHaveBeenCalledTimes(2);
expect(cleanUp).toHaveBeenCalledTimes(1);
});

it('handles ref functions with stable identity', () => {
const container = document.createElement('div');
const cleanUp = jest.fn();
const setup = jest.fn();

function _onRefChange(_ref) {
setup(_ref);
return cleanUp;
}

ReactDOM.render(<div ref={_onRefChange} />, container);

expect(setup).toHaveBeenCalledTimes(1);
expect(cleanUp).toHaveBeenCalledTimes(0);

ReactDOM.render(
<div className="niceClassName" ref={_onRefChange} />,
container,
);

expect(setup).toHaveBeenCalledTimes(1);
expect(cleanUp).toHaveBeenCalledTimes(0);

ReactDOM.render(<div />, container);

expect(setup).toHaveBeenCalledTimes(1);
expect(cleanUp).toHaveBeenCalledTimes(1);
});

it('warns if clean up function is returned when called with null', () => {
const container = document.createElement('div');
const cleanUp = jest.fn();
const setup = jest.fn();
let returnCleanUp = false;

ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
if (returnCleanUp) {
return cleanUp;
}
}}
/>,
container,
);

expect(setup).toHaveBeenCalledTimes(1);
expect(cleanUp).toHaveBeenCalledTimes(0);

returnCleanUp = true;

expect(() => {
ReactDOM.render(
<div
ref={_ref => {
setup(_ref);
if (returnCleanUp) {
return cleanUp;
}
}}
/>,
container,
);
}).toErrorDev('Unexpected return value from a callback ref in div');
});
});
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiber.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ function FiberNode(
this.index = 0;

this.ref = null;
this.refCleanup = null;

this.pendingProps = pendingProps;
this.memoizedProps = null;
Expand Down Expand Up @@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
workInProgress.sibling = current.sibling;
workInProgress.index = current.index;
workInProgress.ref = current.ref;
workInProgress.refCleanup = current.refCleanup;

if (enableProfilerTimer) {
workInProgress.selfBaseDuration = current.selfBaseDuration;
Expand Down Expand Up @@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV(
target.sibling = source.sibling;
target.index = source.index;
target.ref = source.ref;
target.refCleanup = source.refCleanup;
target.pendingProps = source.pendingProps;
target.memoizedProps = source.memoizedProps;
target.updateQueue = source.updateQueue;
Expand Down
3 changes: 3 additions & 0 deletions packages/react-reconciler/src/ReactFiber.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ function FiberNode(
this.index = 0;

this.ref = null;
this.refCleanup = null;

this.pendingProps = pendingProps;
this.memoizedProps = null;
Expand Down Expand Up @@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
workInProgress.sibling = current.sibling;
workInProgress.index = current.index;
workInProgress.ref = current.ref;
workInProgress.refCleanup = current.refCleanup;

if (enableProfilerTimer) {
workInProgress.selfBaseDuration = current.selfBaseDuration;
Expand Down Expand Up @@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV(
target.sibling = source.sibling;
target.index = source.index;
target.ref = source.ref;
target.refCleanup = source.refCleanup;
target.pendingProps = source.pendingProps;
target.memoizedProps = source.memoizedProps;
target.updateQueue = source.updateQueue;
Expand Down
62 changes: 27 additions & 35 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,32 @@ function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {

function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {
const ref = current.ref;
const refCleanup = current.refCleanup;

if (ref !== null) {
if (typeof ref === 'function') {
if (typeof refCleanup === 'function') {
try {
if (shouldProfile(current)) {
try {
startLayoutEffectTimer();
refCleanup();
} finally {
recordLayoutEffectDuration(current);
}
} else {
refCleanup();
}
} catch (error) {
captureCommitPhaseError(current, nearestMountedAncestor, error);
} finally {
// `refCleanup` has been called. Nullify all references to it to prevent double invocation.
current.refCleanup = null;
const finishedWork = current.alternate;
if (finishedWork != null) {
finishedWork.refCleanup = null;
}
}
} else if (typeof ref === 'function') {
let retVal;
try {
if (shouldProfile(current)) {
Expand Down Expand Up @@ -1572,25 +1596,15 @@ function commitAttachRef(finishedWork: Fiber) {
instanceToUse = instance;
}
if (typeof ref === 'function') {
let retVal;
if (shouldProfile(finishedWork)) {
try {
startLayoutEffectTimer();
retVal = ref(instanceToUse);
finishedWork.refCleanup = ref(instanceToUse);
} finally {
recordLayoutEffectDuration(finishedWork);
}
} else {
retVal = ref(instanceToUse);
}
if (__DEV__) {
if (typeof retVal === 'function') {
console.error(
'Unexpected return value from a callback ref in %s. ' +
'A callback ref should not return a function.',
getComponentNameFromFiber(finishedWork),
);
}
finishedWork.refCleanup = ref(instanceToUse);
}
} else {
if (__DEV__) {
Expand All @@ -1609,27 +1623,6 @@ function commitAttachRef(finishedWork: Fiber) {
}
}

function commitDetachRef(current: Fiber) {
const currentRef = current.ref;
if (currentRef !== null) {
if (typeof currentRef === 'function') {
if (shouldProfile(current)) {
try {
startLayoutEffectTimer();
currentRef(null);
} finally {
recordLayoutEffectDuration(current);
}
} else {
currentRef(null);
}
} else {
// $FlowFixMe unable to narrow type to the non-function case
currentRef.current = null;
}
}
}

function detachFiberMutation(fiber: Fiber) {
// Cut off the return pointer to disconnect it from the tree.
// This enables us to detect and warn against state updates on an unmounted component.
Expand Down Expand Up @@ -4450,7 +4443,6 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {
export {
commitPlacement,
commitAttachRef,
commitDetachRef,
invokeLayoutEffectMountInDEV,
invokeLayoutEffectUnmountInDEV,
invokePassiveEffectMountInDEV,
Expand Down
Loading

0 comments on commit e982254

Please sign in to comment.