@samitouri / QOS-React-2 / commits / e98225485a

Add ref cleanup function (#25686)

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 } }} /> ```

Samuel Susla committed Nov 29, 2022 at 16:41 UTC e98225485a124e35abc4cea82e6da944472ce7c7

6 files changed +195 -99

packages/react-dom/src/__tests__/refs-test.js

+133 -29

@@ -401,35 +401,6 @@ describe('ref swapping', () => {

401 'Expected ref to be a function, a string, an object returned by React.createRef(), or null.',

402 );

403 });

404 -

405 - it('should warn about callback refs returning a function', () => {

406 - const container = document.createElement('div');

407 - expect(() => {

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

409 - }).toErrorDev('Unexpected return value from a callback ref in div');

410 -

411 - // Cleanup should warn, too.

412 - expect(() => {

413 - ReactDOM.render(<span />, container);

414 - }).toErrorDev('Unexpected return value from a callback ref in div', {

415 - withoutStack: true,

416 - });

417 -

418 - // No warning when returning non-functions.

419 - ReactDOM.render(<p ref={() => ({})} />, container);

420 - ReactDOM.render(<p ref={() => null} />, container);

421 - ReactDOM.render(<p ref={() => undefined} />, container);

422 -

423 - // Still warns on functions (not deduped).

424 - expect(() => {

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

426 - }).toErrorDev('Unexpected return value from a callback ref in div');

427 - expect(() => {

428 - ReactDOM.unmountComponentAtNode(container);

429 - }).toErrorDev('Unexpected return value from a callback ref in div', {

430 - withoutStack: true,

431 - });

432 - });

404 });

405

406 describe('root level refs', () => {

@@ -612,3 +583,136 @@ describe('strings refs across renderers', () => {

583 expect(inst.refs.child2).toBe(div2.firstChild);

584 });

585 });

586 +

587 +describe('refs return clean up function', () => {

588 + it('calls clean up function if it exists', () => {

589 + const container = document.createElement('div');

590 + let cleanUp = jest.fn();

591 + let setup = jest.fn();

592 +

593 + ReactDOM.render(

594 + <div

595 + ref={_ref => {

596 + setup(_ref);

597 + return cleanUp;

598 + }}

599 + />,

600 + container,

601 + );

602 +

603 + ReactDOM.render(

604 + <div

605 + ref={_ref => {

606 + setup(_ref);

607 + }}

608 + />,

609 + container,

610 + );

611 +

612 + expect(setup).toHaveBeenCalledTimes(2);

613 + expect(cleanUp).toHaveBeenCalledTimes(1);

614 + expect(cleanUp.mock.calls[0][0]).toBe(undefined);

615 +

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

617 +

618 + expect(cleanUp).toHaveBeenCalledTimes(1);

619 + expect(setup).toHaveBeenCalledTimes(3);

620 + expect(setup.mock.calls[2][0]).toBe(null);

621 +

622 + cleanUp = jest.fn();

623 + setup = jest.fn();

624 +

625 + ReactDOM.render(

626 + <div

627 + ref={_ref => {

628 + setup(_ref);

629 + return cleanUp;

630 + }}

631 + />,

632 + container,

633 + );

634 +

635 + expect(setup).toHaveBeenCalledTimes(1);

636 + expect(cleanUp).toHaveBeenCalledTimes(0);

637 +

638 + ReactDOM.render(

639 + <div

640 + ref={_ref => {

641 + setup(_ref);

642 + return cleanUp;

643 + }}

644 + />,

645 + container,

646 + );

647 +

648 + expect(setup).toHaveBeenCalledTimes(2);

649 + expect(cleanUp).toHaveBeenCalledTimes(1);

650 + });

651 +

652 + it('handles ref functions with stable identity', () => {

653 + const container = document.createElement('div');

654 + const cleanUp = jest.fn();

655 + const setup = jest.fn();

656 +

657 + function _onRefChange(_ref) {

658 + setup(_ref);

659 + return cleanUp;

660 + }

661 +

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

663 +

664 + expect(setup).toHaveBeenCalledTimes(1);

665 + expect(cleanUp).toHaveBeenCalledTimes(0);

666 +

667 + ReactDOM.render(

668 + <div className="niceClassName" ref={_onRefChange} />,

669 + container,

670 + );

671 +

672 + expect(setup).toHaveBeenCalledTimes(1);

673 + expect(cleanUp).toHaveBeenCalledTimes(0);

674 +

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

676 +

677 + expect(setup).toHaveBeenCalledTimes(1);

678 + expect(cleanUp).toHaveBeenCalledTimes(1);

679 + });

680 +

681 + it('warns if clean up function is returned when called with null', () => {

682 + const container = document.createElement('div');

683 + const cleanUp = jest.fn();

684 + const setup = jest.fn();

685 + let returnCleanUp = false;

686 +

687 + ReactDOM.render(

688 + <div

689 + ref={_ref => {

690 + setup(_ref);

691 + if (returnCleanUp) {

692 + return cleanUp;

693 + }

694 + }}

695 + />,

696 + container,

697 + );

698 +

699 + expect(setup).toHaveBeenCalledTimes(1);

700 + expect(cleanUp).toHaveBeenCalledTimes(0);

701 +

702 + returnCleanUp = true;

703 +

704 + expect(() => {

705 + ReactDOM.render(

706 + <div

707 + ref={_ref => {

708 + setup(_ref);

709 + if (returnCleanUp) {

710 + return cleanUp;

711 + }

712 + }}

713 + />,

714 + container,

715 + );

716 + }).toErrorDev('Unexpected return value from a callback ref in div');

717 + });

718 +});

packages/react-reconciler/src/ReactFiber.new.js

@@ -148,6 +148,7 @@ function FiberNode(

148 this.index = 0;

149

150 this.ref = null;

151 + this.refCleanup = null;

152

153 this.pendingProps = pendingProps;

154 this.memoizedProps = null;

@@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {

337 workInProgress.sibling = current.sibling;

338 workInProgress.index = current.index;

339 workInProgress.ref = current.ref;

340 + workInProgress.refCleanup = current.refCleanup;

341

342 if (enableProfilerTimer) {

343 workInProgress.selfBaseDuration = current.selfBaseDuration;

@@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV(

882 target.sibling = source.sibling;

883 target.index = source.index;

884 target.ref = source.ref;

885 + target.refCleanup = source.refCleanup;

886 target.pendingProps = source.pendingProps;

887 target.memoizedProps = source.memoizedProps;

888 target.updateQueue = source.updateQueue;

packages/react-reconciler/src/ReactFiber.old.js

@@ -148,6 +148,7 @@ function FiberNode(

148 this.index = 0;

149

150 this.ref = null;

151 + this.refCleanup = null;

152

153 this.pendingProps = pendingProps;

154 this.memoizedProps = null;

@@ -336,6 +337,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {

337 workInProgress.sibling = current.sibling;

338 workInProgress.index = current.index;

339 workInProgress.ref = current.ref;

340 + workInProgress.refCleanup = current.refCleanup;

341

342 if (enableProfilerTimer) {

343 workInProgress.selfBaseDuration = current.selfBaseDuration;

@@ -880,6 +882,7 @@ export function assignFiberPropertiesInDEV(

882 target.sibling = source.sibling;

883 target.index = source.index;

884 target.ref = source.ref;

885 + target.refCleanup = source.refCleanup;

886 target.pendingProps = source.pendingProps;

887 target.memoizedProps = source.memoizedProps;

888 target.updateQueue = source.updateQueue;

packages/react-reconciler/src/ReactFiberCommitWork.new.js

+27 -35

@@ -286,8 +286,32 @@ function safelyAttachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {

286

287 function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) {

288 const ref = current.ref;

289 + const refCleanup = current.refCleanup;

290 +

291 if (ref !== null) {

290 - if (typeof ref === 'function') {

292 + if (typeof refCleanup === 'function') {

293 + try {

294 + if (shouldProfile(current)) {

295 + try {

296 + startLayoutEffectTimer();

297 + refCleanup();

298 + } finally {

299 + recordLayoutEffectDuration(current);

300 + }

301 + } else {

302 + refCleanup();

303 + }

304 + } catch (error) {

305 + captureCommitPhaseError(current, nearestMountedAncestor, error);

306 + } finally {

307 + // `refCleanup` has been called. Nullify all references to it to prevent double invocation.

308 + current.refCleanup = null;

309 + const finishedWork = current.alternate;

310 + if (finishedWork != null) {

311 + finishedWork.refCleanup = null;

312 + }

313 + }

314 + } else if (typeof ref === 'function') {

315 let retVal;

316 try {

317 if (shouldProfile(current)) {

@@ -1572,25 +1596,15 @@ function commitAttachRef(finishedWork: Fiber) {

1596 instanceToUse = instance;

1597 }

1598 if (typeof ref === 'function') {

1575 - let retVal;

1599 if (shouldProfile(finishedWork)) {

1600 try {

1601 startLayoutEffectTimer();

1579 - retVal = ref(instanceToUse);

1602 + finishedWork.refCleanup = ref(instanceToUse);

1603 } finally {

1604 recordLayoutEffectDuration(finishedWork);

1605 }

1606 } else {

1584 - retVal = ref(instanceToUse);

1585 - }

1586 - if (__DEV__) {

1587 - if (typeof retVal === 'function') {

1588 - console.error(

1589 - 'Unexpected return value from a callback ref in %s. ' +

1590 - 'A callback ref should not return a function.',

1591 - getComponentNameFromFiber(finishedWork),

1592 - );

1593 - }

1607 + finishedWork.refCleanup = ref(instanceToUse);

1608 }

1609 } else {

1610 if (__DEV__) {

@@ -1609,27 +1623,6 @@ function commitAttachRef(finishedWork: Fiber) {

1623 }

1624 }

1625

1612 -function commitDetachRef(current: Fiber) {

1613 - const currentRef = current.ref;

1614 - if (currentRef !== null) {

1615 - if (typeof currentRef === 'function') {

1616 - if (shouldProfile(current)) {

1617 - try {

1618 - startLayoutEffectTimer();

1619 - currentRef(null);

1620 - } finally {

1621 - recordLayoutEffectDuration(current);

1622 - }

1623 - } else {

1624 - currentRef(null);

1625 - }

1626 - } else {

1627 - // $FlowFixMe unable to narrow type to the non-function case

1628 - currentRef.current = null;

1629 - }

1630 - }

1631 -}

1632 -

1626 function detachFiberMutation(fiber: Fiber) {

1627 // Cut off the return pointer to disconnect it from the tree.

1628 // This enables us to detect and warn against state updates on an unmounted component.

@@ -4450,7 +4443,6 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void {

4443 export {

4444 commitPlacement,

4445 commitAttachRef,

4453 - commitDetachRef,

4446 invokeLayoutEffectMountInDEV,

4447 invokeLayoutEffectUnmountInDEV,

4448 invokePassiveEffectMountInDEV,

packages/react-reconciler/src/ReactFiberCommitWork.old.js