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

docs: add attention #2274

Merged
merged 1 commit into from
Aug 3, 2023
Merged

docs: add attention #2274

merged 1 commit into from
Aug 3, 2023

Conversation

hchlq
Copy link
Collaborator

@hchlq hchlq commented Aug 2, 2023

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

fix #2259

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@hchlq hchlq requested a review from crazylxr August 2, 2023 02:09
@Woodii1998
Copy link

@hchlq 这个 pr 似乎并没有修到问题的关键 我看了一下代码 这个问题的产生是 如果外部传入的 list 在没有经过状态管理的情况下

https://github.com/alibaba/hooks/blob/d65713aa162d2b58f29bd8a09e4d0f8b40d20cd2/packages/hooks/src/useVirtualList/index.ts#L105C24-L105C24

https://github.com/alibaba/hooks/blob/d65713aa162d2b58f29bd8a09e4d0f8b40d20cd2/packages/hooks/src/useVirtualList/index.ts#L110C8-L110C8

这两处的状态修改会导致页面重新渲染,进而导致 list 传入一个新的数组,所以哪怕数组不为空 也会导致这个问题, 这个例子应该可以更好的诠释这个问题

https://codesandbox.io/s/default-usage-forked-dtsr4l?file=/App.tsx

image

所以我觉得应该在修改这两个状态的时候判断一下修改前后的值是否相同来修复这个问题

https://github.com/alibaba/hooks/blob/d65713aa162d2b58f29bd8a09e4d0f8b40d20cd2/packages/hooks/src/useVirtualList/index.ts#L105C24-L105C24

https://github.com/alibaba/hooks/blob/d65713aa162d2b58f29bd8a09e4d0f8b40d20cd2/packages/hooks/src/useVirtualList/index.ts#L110C8-L110C8

或者你们有什么更好的想法

@hchlq
Copy link
Collaborator Author

hchlq commented Aug 2, 2023

我理解你意思了,我还以为是空数组导致的,那是这个问题的话,这个改动破环性有点大,建议先不做改动。建议业务侧做 memo 处理

原因:list 的变化一直触发重新计算

  useEffect(() => {
    if (!size?.width || !size?.height) {
      return;
    }
    calculateRange();
  }, [size?.width, size?.height, list]);

@hchlq
Copy link
Collaborator Author

hchlq commented Aug 2, 2023

本次先增加文档提示,暂先不解决这个问题

@hchlq hchlq changed the title fix(useVirtualList): empty list cause endless loop docs: add attention Aug 2, 2023
@Woodii1998
Copy link

@hchlq 我觉得文档是要更新的,但是对于 hook 内部 这样处理一下你觉得怎么样 ⬆

@crazylxr crazylxr merged commit 6579130 into alibaba:master Aug 3, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants