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: should cleanup styles in useEffect cleanup function #141

Merged
merged 6 commits into from
Aug 10, 2023

Conversation

MadCcc
Copy link
Member

@MadCcc MadCcc commented Aug 9, 2023

React 18 启用 useInsertionEffect 时,切换主题的场景下,样式会先于 token 写入 cache 并插入 dom,因此在某些场景下(比如这个 PR 的测试用例),最新的样式会被 token 卸载清除。

解法:把清除 token 样式的逻辑全部放在 useEffect 的 cleanup 方法中执行,这样会保证在渲染的最后执行清除逻辑。

@github-actions
Copy link

github-actions bot commented Aug 9, 2023

😭 Deploy PR Preview b79520f failed. Build logs

🤖 By surge-preview

}

React.useEffect(() => {
// Compatible with strict mode
Copy link
Member

Choose a reason for hiding this comment

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

这个不行,strictmode 行为是这样子的,但是 Suspence 里可能是同一个 render 跑两次 effect。

Copy link
Member Author

Choose a reason for hiding this comment

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

同一个 render 两次 effect 一次 cleanup 的话这里也不影响了,目的是能在非 cleanup 阶段注册就行

@MadCcc MadCcc merged commit c682ea6 into master Aug 10, 2023
6 checks passed
@MadCcc MadCcc deleted the fix/cleanup-in-effect branch August 10, 2023 02:23
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.

2 participants