fix: ordering issue with Component re-render (#4125)

preactjs · Sep 3, 2023 · 7a3706a · 7a3706a
1 parent 317025f
commit 7a3706a
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 1 deletion.
diff --git a/src/component.js b/src/component.js
@@ -103,7 +103,7 @@ export function getDomSibling(vnode, childIndex) {
 			// Since updateParentDomPointers keeps _dom pointer correct,
 			// we can rely on _dom to tell us if this subtree contains a
 			// rendered DOM node, and what the first rendered DOM node is
-			return sibling._dom;
+			return sibling._nextDom || sibling._dom;
 		}
 	}
 

diff --git a/src/diff/children.js b/src/diff/children.js
@@ -111,6 +111,7 @@ export function diffChildren(
 			oldVNode = oldChildren[i];
 			if (oldVNode && oldVNode.key == null && oldVNode._dom) {
 				if (oldVNode._dom == oldDom) {
+					oldVNode._parent = oldParentVNode;
 					oldDom = getDomSibling(oldVNode);
 				}
 

diff --git a/test/browser/fragments.test.js b/test/browser/fragments.test.js
@@ -646,6 +646,110 @@ describe('Fragment', () => {
 		expect(scratch.innerHTML).to.equal('<div>Hello</div>');
 	});
 
+	it('should preserve order for fragment switching', () => {
+		let set;
+		class Foo extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { isLoading: true, data: null };
+				set = this.setState.bind(this);
+			}
+			render(props, { isLoading, data }) {
+				return (
+					<Fragment>
+						<div>HEADER</div>
+						{isLoading ? <div>Loading...</div> : null}
+						{data ? <div>Content: {data}</div> : null}
+					</Fragment>
+				);
+			}
+		}
+
+		render(<Foo />, scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Loading...</div>'
+		);
+
+		set({ isLoading: false, data: 2 });
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Content: 2</div>'
+		);
+	});
+
+	it('should preserve order for nested fragment switching w/ child return', () => {
+		let set;
+		const Wrapper = ({ children }) => <Fragment>{children}</Fragment>;
+		class Foo extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { isLoading: true, data: null };
+				set = this.setState.bind(this);
+			}
+			render(props, { isLoading, data }) {
+				return (
+					<Fragment>
+						<div>HEADER</div>
+						{isLoading ? <div>Loading...</div> : null}
+						{data ? <div>Content: {data}</div> : null}
+					</Fragment>
+				);
+			}
+		}
+
+		render(
+			<Wrapper>
+				<Foo />
+			</Wrapper>,
+			scratch
+		);
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Loading...</div>'
+		);
+
+		set({ isLoading: false, data: 2 });
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Content: 2</div>'
+		);
+	});
+
+	it('should preserve order for nested fragment switching', () => {
+		let set;
+		const Wrapper = () => (
+			<Fragment>
+				<Foo />
+			</Fragment>
+		);
+		class Foo extends Component {
+			constructor(props) {
+				super(props);
+				this.state = { isLoading: true, data: null };
+				set = this.setState.bind(this);
+			}
+			render(props, { isLoading, data }) {
+				return (
+					<Fragment>
+						<div>HEADER</div>
+						{isLoading ? <div>Loading...</div> : null}
+						{data ? <div>Content: {data}</div> : null}
+					</Fragment>
+				);
+			}
+		}
+
+		render(<Wrapper />, scratch);
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Loading...</div>'
+		);
+
+		set({ isLoading: false, data: 2 });
+		rerender();
+		expect(scratch.innerHTML).to.equal(
+			'<div>HEADER</div><div>Content: 2</div>'
+		);
+	});
+
 	it('should preserve state with reordering in multiple levels', () => {
 		function Foo({ condition }) {
 			return condition ? (