Remove prevRef from internal (#3825)

src/diff/children.js

@@ -29,6 +29,7 @@ export function patchChildren(internal, children, parentDom) {
 	let skew = 0;
 	let i;
 
+	let refs;
 	/** @type {import('../internal').Internal} */
 	let childInternal;
 
@@ -71,12 +72,20 @@ export function patchChildren(internal, children, parentDom) {
 		if (mountingChild) {
 			childInternal = createInternal(childVNode, internal);
 
+			if (!refs) refs = [];
+
+			if (childVNode.ref) {
+				refs.push(childInternal, undefined);
+				childInternal.ref = childVNode.ref;
+			}
+
 			// We are mounting a new VNode
 			mount(
 				childInternal,
 				childVNode,
 				parentDom,
-				getDomSibling(internal, skewedIndex)
+				getDomSibling(internal, skewedIndex),
+				refs
 			);
 		}
 		// If this node suspended during hydration, and no other flags are set:
@@ -85,9 +94,22 @@ export function patchChildren(internal, children, parentDom) {
 			(childInternal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) ===
 			(MODE_HYDRATE | MODE_SUSPENDED)
 		) {
+			if (!refs) refs = [];
+
+			if (childVNode.ref) {
+				refs.push(childInternal, undefined);
+				childInternal.ref = childVNode.ref;
+			}
+
 			// We are resuming the hydration of a VNode
 			mount(childInternal, childVNode, parentDom, childInternal.data);
 		} else {
+			if (childInternal.ref != childVNode.ref) {
+				if (!refs) refs = [];
+				refs.push(childInternal, childInternal.ref);
+				childInternal.ref = childVNode.ref;
+			}
+
 			// Morph the old element into the new one, but don't append it to the dom yet
 			patch(childInternal, childVNode, parentDom);
 		}
@@ -154,19 +176,12 @@ export function patchChildren(internal, children, parentDom) {
 	}
 
 	// Set refs only after unmount
-	for (i = 0; i < newChildren.length; i++) {
-		childInternal = newChildren[i];
-		if (childInternal) {
-			let oldRef = childInternal._prevRef;
-			if (childInternal.ref != oldRef) {
-				if (oldRef) applyRef(oldRef, null, childInternal);
-				if (childInternal.ref)
-					applyRef(
-						childInternal.ref,
-						childInternal._component || childInternal.data,
-						childInternal
-					);
-			}
+	if (refs) {
+		for (i = 0; i < refs.length; i += 2) {
+			const internal = refs[i];
+			if (refs[i + 1]) applyRef(refs[i + 1], null, internal);
+			if (internal && internal.ref)
+				applyRef(internal.ref, internal._component || internal.data, internal);
 		}
 	}
 }

src/diff/mount.js

@@ -27,7 +27,7 @@ import { commitQueue } from './commit';
  * @param {import('../internal').PreactNode} startDom
  * @returns {import('../internal').PreactNode | null} pointer to the next DOM node to be hydrated (or null)
  */
-export function mount(internal, newVNode, parentDom, startDom) {
+export function mount(internal, newVNode, parentDom, startDom, refs) {
 	if (options._diff) options._diff(internal, newVNode);
 
 	/** @type {import('../internal').PreactNode} */
@@ -55,7 +55,8 @@ export function mount(internal, newVNode, parentDom, startDom) {
 					internal,
 					renderResult,
 					parentDom,
-					startDom
+					startDom,
+					refs
 				);
 			}
 
@@ -72,7 +73,7 @@ export function mount(internal, newVNode, parentDom, startDom) {
 					? startDom
 					: null;
 
-			nextDomSibling = mountElement(internal, hydrateDom);
+			nextDomSibling = mountElement(internal, hydrateDom, refs);
 		}
 
 		if (options.diffed) options.diffed(internal);
@@ -100,7 +101,7 @@ export function mount(internal, newVNode, parentDom, startDom) {
  * @param {import('../internal').PreactNode} dom A DOM node to attempt to re-use during hydration
  * @returns {import('../internal').PreactNode}
  */
-function mountElement(internal, dom) {
+function mountElement(internal, dom, refs) {
 	let newProps = internal.props;
 	let nodeType = internal.type;
 	let flags = internal.flags;
@@ -202,7 +203,8 @@ function mountElement(internal, dom) {
 				internal,
 				Array.isArray(newChildren) ? newChildren : [newChildren],
 				dom,
-				isNew ? null : dom.firstChild
+				isNew ? null : dom.firstChild,
+				refs
 			);
 		}
 
@@ -223,7 +225,7 @@ function mountElement(internal, dom) {
  * @param {import('../internal').PreactElement} parentDom The element into which this subtree is rendered
  * @param {import('../internal').PreactNode} startDom
  */
-export function mountChildren(internal, children, parentDom, startDom) {
+export function mountChildren(internal, children, parentDom, startDom, refs) {
 	let internalChildren = (internal._children = []),
 		i,
 		childVNode,
@@ -262,11 +264,16 @@ export function mountChildren(internal, children, parentDom, startDom) {
 		}
 
 		if (childInternal.ref) {
-			applyRef(
-				childInternal.ref,
-				childInternal._component || newDom,
-				childInternal
-			);
+			if (refs) {
+				refs.push(childInternal, undefined);
+				childInternal.ref = childVNode.ref;
+			} else {
+				applyRef(
+					childInternal.ref,
+					childInternal._component || newDom,
+					childInternal
+				);
+			}
 		}
 	}
 

src/diff/patch.js

@@ -127,9 +127,6 @@ export function patch(internal, vnode, parentDom) {
 
 	// Once we have successfully rendered the new VNode, copy it's ID over
 	internal._vnodeId = vnode._vnodeId;
-
-	internal._prevRef = internal.ref;
-	internal.ref = vnode.ref;
 }
 
 /**

src/internal.d.ts

@@ -136,7 +136,6 @@ export interface Internal<P = {}> {
 	props: (P & { children: ComponentChildren }) | string | number;
 	key: any;
 	ref: Ref<any> | null;
-	_prevRef: Ref<any> | null;
 
 	/** Bitfield containing information about the Internal or its component. */
 	flags: number;

src/tree.js

@@ -86,7 +86,6 @@ export function createInternal(vnode, parentInternal) {
 		props,
 		key,
 		ref,
-		_prevRef: null,
 		data:
 			flags & TYPE_COMPONENT
 				? { _commitCallbacks: [], _context: null, _stateCallbacks: [] }

test/browser/refs.test.js

@@ -415,8 +415,7 @@ describe('refs', () => {
 		expect(input.value).to.equal('foo');
 	});
 
-	// @TODO: re-enable these tests when we add a ref queue
-	it.skip('should correctly set nested child refs', () => {
+	it('should correctly set nested child refs', () => {
 		const ref = createRef();
 		const App = ({ open }) =>
 			open ? (
@@ -436,7 +435,7 @@ describe('refs', () => {
 		expect(ref.current).to.not.be.null;
 	});
 
-	it.skip('should correctly set nested child function refs', () => {
+	it('should correctly set nested child function refs', () => {
 		const ref = sinon.spy();
 		const App = ({ open }) =>
 			open ? (
@@ -457,7 +456,7 @@ describe('refs', () => {
 		ref.resetHistory();
 
 		render(<App open />, scratch);
-		expect(ref).to.have.been.calledOnce.and.calledWith(
+		expect(ref).to.have.been.calledTwice.and.calledWith(
 			scratch.firstElementChild.firstElementChild
 		);
 	});
@@ -537,7 +536,7 @@ describe('refs', () => {
 		expect(ref.current.innerHTML).to.be.equal('Foo');
 	});
 
-	it.skip('should not remove refs for memoized components keyed', () => {
+	it('should not remove refs for memoized components keyed', () => {
 		const ref = createRef();
 		const element = <div ref={ref}>hey</div>;
 		function App(props) {
@@ -567,22 +566,23 @@ describe('refs', () => {
 		expect(ref.current).to.equal(scratch.firstChild.firstChild);
 	});
 
-	// TODO
-	it.skip('should properly call null for memoized components keyed', () => {
+	it('should properly call null for memoized components keyed', () => {
 		const calls = [];
 		const element = <div ref={x => calls.push(x)}>hey</div>;
 		function App(props) {
 			return <div key={props.count}>{element}</div>;
 		}
 
 		render(<App count={0} />, scratch);
+		const firstChildAfterFirstRender = scratch.firstChild.firstChild;
 		render(<App count={1} />, scratch);
+		const firstChildAfterSecondRender = scratch.firstChild.firstChild;
 		render(<App count={2} />, scratch);
 		expect(calls.length).to.equal(5);
 		expect(calls).to.deep.equal([
-			scratch.firstChild.firstChild,
+			firstChildAfterFirstRender,
 			null,
-			scratch.firstChild.firstChild,
+			firstChildAfterSecondRender,
 			null,
 			scratch.firstChild.firstChild
 		]);