Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix recordHostChildrenToDelete for fragment deletion #43

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

Russellwzr
Copy link

原代码中标记Fragment下需要被删除的元素时,没有考虑到Fragment嵌套的情况,会导致部分待删除元素被忽略,可用原代码运行如下组件进行复现:

function App() {
	const [num, setNum] = useState(0);

	return (
		<ul onClick={() => setNum((prevNum) => prevNum + 1)}>
			<li>a</li>
			<li>b</li>
			{num % 2 === 0 ? (
				<>
					<li>item-1</li>
					<>
						<li>item-2</li>
						<>
							<li>item-3</li>
							<li>item-4</li>
						</>
						<li>item-5</li>
					</>
					<li>item</li>
				</>
			) : (
				<li>test</li>
			)}
			<li>c</li>
			<li>d</li>
		</ul>
	);
}

改进策略为:修改recordHostChildrenToDelete,通过BFS标记各层Fragment下需要删除的元素,并将recordHostChildrenToDelete与commitNestedUnmounts解耦,增强代码可读性。

}
node = node.sibling;
function recordHostChildrenToDelete(beginNode: FiberNode): FiberNode[] {
if (beginNode.tag !== Fragment) return [beginNode];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recordHostChildrenToDelete方法应该是找到第一层Host类型节点,这里的实现看起来是找到第一层“非Fragment”类型节点?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感谢提醒,当时确实只把关注点落在了修复多层级的Fragment上而忽略了其他情况,额外加上Function Component的判断应该就可以了,本地又测试了下~

}
node = node.sibling;
function recordHostChildrenToDelete(beginNode: FiberNode): FiberNode[] {
if (beginNode.tag !== Fragment && beginNode.tag !== FunctionComponent)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

相比于 判断 !== ,可以抽一个方法判断当前节点是 Host类型,类似:

function isHostTypeFiberNode(fiber: FiberNode) {
  const tag = fiber.tag;
  return [HostComponent, HostRoot, HostText].includes(tag);
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯对的,确实可以单抽出来方便后面复用

const hostChildrenToDelete: FiberNode[] =
recordHostChildrenToDelete(childToDelete);

for (let i = 0; i < hostChildrenToDelete.length; i++) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的实现似乎有些问题,不应该commitNestedUnmounts(hostChildrenToDelete[i],这样会丢失一些非Host类型组件的遍历(比如FC

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

对的,还是一开始关注点偏离了FC的问题,这里需要恢复传参为childToDelete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants