-
Notifications
You must be signed in to change notification settings - Fork 167
perf: skipping re-rendering #190
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
base: master
Are you sure you want to change the base?
Changes from all commits
035d0e2
03d7e0d
50985c2
3dd9238
46ee6b4
2a00a14
a6914a1
e82a20d
3fa5f31
6a3ee26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,7 @@ import useOriginScroll from './hooks/useOriginScroll'; | |
import useScrollDrag from './hooks/useScrollDrag'; | ||
import type { ScrollPos, ScrollTarget } from './hooks/useScrollTo'; | ||
import useScrollTo from './hooks/useScrollTo'; | ||
import type { ExtraRenderInfo, GetKey, RenderFunc, SharedConfig } from './interface'; | ||
import type { ExtraRenderInfo, GetKey, RenderFunc } from './interface'; | ||
import type { ScrollBarDirectionType, ScrollBarRef } from './ScrollBar'; | ||
import ScrollBar from './ScrollBar'; | ||
import { getSpinSize } from './utils/scrollbarUtil'; | ||
|
@@ -165,10 +165,6 @@ export function RawList<T>(props: ListProps<T>, ref: React.Ref<ListRef>) { | |
setScrollMoving(false); | ||
}; | ||
|
||
const sharedConfig: SharedConfig<T> = { | ||
getKey, | ||
}; | ||
|
||
// ================================ Scroll ================================ | ||
function syncScrollTop(newTop: number | ((prev: number) => number)) { | ||
setOffsetTop((origin) => { | ||
|
@@ -572,7 +568,7 @@ export function RawList<T>(props: ListProps<T>, ref: React.Ref<ListRef>) { | |
offsetLeft, | ||
setInstanceRef, | ||
children, | ||
sharedConfig, | ||
getKey, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainuseChildren 调用已改为传入 getKey;请检查仓库内是否仍有 SharedConfig 遗留引用 为避免类型/行为不一致,请全局确认:
🏁 Script executed: #!/bin/bash
# 1) 是否仍有 SharedConfig 引用
rg -nP --glob '!**/node_modules/**' '\bSharedConfig<'
# 2) 检查 useChildren 的调用签名(查看上下文)
rg -nP --glob '!**/node_modules/**' 'useChildren\s*\(' -C2
# 3) 确认 Item.tsx 的 useCallback 依赖是否包含 setRef
rg -nP --glob '!**/node_modules/**' '(export\s+function\s+Item\b|const\s+Item\s*=)' -n -C5 src/Item.tsx Length of output: 600 修复:为 src/Item.tsx 中的 useCallback 添加 setRef 依赖;同时全局确认 SharedConfig/useChildren 变更
🤖 Prompt for AI Agents
|
||
); | ||
|
||
let componentStyle: React.CSSProperties = null; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||
import * as React from 'react'; | ||||||||||||||||||||||||||||||||
import type { RenderFunc, SharedConfig } from '../interface'; | ||||||||||||||||||||||||||||||||
import type { RenderFunc, GetKey } from '../interface'; | ||||||||||||||||||||||||||||||||
import { Item } from '../Item'; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
export default function useChildren<T>( | ||||||||||||||||||||||||||||||||
|
@@ -10,22 +10,25 @@ export default function useChildren<T>( | |||||||||||||||||||||||||||||||
offsetX: number, | ||||||||||||||||||||||||||||||||
setNodeRef: (item: T, element: HTMLElement) => void, | ||||||||||||||||||||||||||||||||
renderFunc: RenderFunc<T>, | ||||||||||||||||||||||||||||||||
{ getKey }: SharedConfig<T>, | ||||||||||||||||||||||||||||||||
getKey: GetKey<T>, | ||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||
return list.slice(startIndex, endIndex + 1).map((item, index) => { | ||||||||||||||||||||||||||||||||
const eleIndex = startIndex + index; | ||||||||||||||||||||||||||||||||
const node = renderFunc(item, eleIndex, { | ||||||||||||||||||||||||||||||||
style: { | ||||||||||||||||||||||||||||||||
width: scrollWidth, | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
offsetX, | ||||||||||||||||||||||||||||||||
}) as React.ReactElement; | ||||||||||||||||||||||||||||||||
// The list reference may remain unchanged, but its internal data may change, which can result in different behavior compared to the previous implementation. | ||||||||||||||||||||||||||||||||
return React.useMemo(() => { | ||||||||||||||||||||||||||||||||
return list.slice(startIndex, endIndex + 1).map((item, index) => { | ||||||||||||||||||||||||||||||||
const eleIndex = startIndex + index; | ||||||||||||||||||||||||||||||||
const node = renderFunc(item, eleIndex, { | ||||||||||||||||||||||||||||||||
style: { | ||||||||||||||||||||||||||||||||
width: scrollWidth, | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
offsetX, | ||||||||||||||||||||||||||||||||
}) as React.ReactElement; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
const key = getKey(item); | ||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||
<Item key={key} setRef={(ele) => setNodeRef(item, ele)}> | ||||||||||||||||||||||||||||||||
{node} | ||||||||||||||||||||||||||||||||
</Item> | ||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
const key = getKey(item); | ||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||
<Item key={key} setRef={(ele) => setNodeRef(item, ele)}> | ||||||||||||||||||||||||||||||||
{node} | ||||||||||||||||||||||||||||||||
</Item> | ||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||
Comment on lines
+26
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainsetRef 闭包与 Item 中 useCallback([]) 组合可能导致使用过期的 setRef 此处每次渲染都会生成新的 针对 export function Item({ children, setRef }: ItemProps) {
- const refFunc = React.useCallback(node => {
- setRef(node);
- }, []);
+ const refFunc = React.useCallback((node) => {
+ setRef(node);
+ }, [setRef]);
return React.cloneElement(children, {
ref: refFunc,
});
} 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing repo root:"
ls -la || true
# Find Item.tsx and useChildren.tsx files
mapfile -t ITEM_FILES < <(find . -type f -iname '*Item.tsx' 2>/dev/null || true)
mapfile -t USE_FILES < <(find . -type f -iname 'usechildren.tsx' -o -iname '*useChildren.tsx' 2>/dev/null || true)
echo "Found Item files (${#ITEM_FILES[@]}):"
printf '%s\n' "${ITEM_FILES[@]}"
echo "Found useChildren files (${#USE_FILES[@]}):"
printf '%s\n' "${USE_FILES[@]}"
if [ ${#ITEM_FILES[@]} -gt 0 ]; then
for f in "${ITEM_FILES[@]}"; do
echo
echo "==== FILE: $f ===="
sed -n '1,240p' "$f" || true
echo "==== end $f ===="
done
else
echo "No Item.tsx files found."
fi
if [ ${#USE_FILES[@]} -gt 0 ]; then
for f in "${USE_FILES[@]}"; do
echo
echo "==== FILE: $f ===="
sed -n '1,240p' "$f" || true
echo "==== end $f ===="
done
else
echo "No useChildren.tsx files found."
fi
echo
echo "Searching repository for occurrences of 'setRef' and 'setNodeRef' (showing up to 200 matches):"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "setRef" || true
rg -n --hidden --no-ignore -S "setNodeRef" || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "setRef" . || true
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "setNodeRef" . || true
fi
echo
echo "If Item.tsx was found, searching for useCallback usage inside it:"
for f in "${ITEM_FILES[@]}"; do
echo "---- $f ----"
if command -v rg >/dev/null 2>&1; then
rg -n "useCallback" "$f" || true
rg -n "useCallback\(" "$f" || true
rg -n "useCallback\([^)]*\)\s*,\s*\[\s*\]" "$f" || true
else
grep -n "useCallback" "$f" || true
fi
done
echo
echo "Done." Length of output: 1392 重新运行脚本以定位并打印 src/Item.tsx 与 src/hooks/useChildren.tsx 内容,并搜索 setRef / setNodeRef / useCallback 使用位置: 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Listing src top-level:"
ls -la src || true
# Check a few likely exact paths first
candidates=(
"src/Item.tsx"
"src/Item.jsx"
"src/components/Item.tsx"
"src/components/Item.jsx"
"src/hooks/Item.tsx"
"src/hooks/useChildren.tsx"
"src/hooks/usechildren.tsx"
"src/hooks/useChildren.jsx"
"src/hooks/useChildren.ts"
)
for p in "${candidates[@]}"; do
if [ -f "$p" ]; then
echo
echo "==== FOUND: $p ===="
sed -n '1,240p' "$p" || true
echo "==== end $p ===="
fi
done
echo
echo "Finding any *Item.* and *useChildren.* under src (paths):"
find src -type f \( -iname '*Item.*' -o -iname '*useChildren.*' -o -iname '*usechildren.*' \) -print || true
echo
echo "Printing contents of any matching Item.tsx / useChildren.tsx (first 240 lines):"
find src -type f \( -iname '*Item.tsx' -o -iname '*useChildren.tsx' -o -iname '*usechildren.tsx' \) -print0 2>/dev/null | while IFS= read -r -d '' file; do
echo
echo "---- $file ----"
sed -n '1,240p' "$file" || true
echo "---- end $file ----"
done
echo
echo "Searching repository (src/) for usages of 'setRef', 'setNodeRef' and 'useCallback':"
if command -v rg >/dev/null 2>&1; then
rg -n --hidden --no-ignore -S "setRef" src || true
rg -n --hidden --no-ignore -S "setNodeRef" src || true
rg -n --hidden --no-ignore -S "useCallback" src || true
else
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "setRef" src || true
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "setNodeRef" src || true
grep -RIn --exclude-dir=node_modules --exclude-dir=.git "useCallback" src || true
fi
echo
echo "Done." Length of output: 5261 修复:Item.tsx 的 ref 回调捕获过期的 setRef,需将 useCallback 依赖改为 [setRef] Item 中 refFunc 的 useCallback 使用空依赖,会固定首次传入的 setRef;useChildren 向 Item 传入的 setRef 为内联函数 位置:src/Item.tsx export function Item({ children, setRef }: ItemProps) {
- const refFunc = React.useCallback(node => {
- setRef(node);
- }, []);
+ const refFunc = React.useCallback((node) => {
+ setRef(node);
+ }, [setRef]);
return React.cloneElement(children, {
ref: refFunc,
});
} 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
}, [list, startIndex, endIndex, setNodeRef, renderFunc, getKey, offsetX, scrollWidth]); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 稍微有点复杂了,未来如果加 props 也容易 break。如果是只是为了 Item 不需要重新渲染,可以考虑 Item 直接用 React.memo 包一下,条件里直接忽略 setRef 即可。 |
||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -24,11 +24,11 @@ export default function useHeights<T>( | |||||
|
||||||
const promiseIdRef = useRef<number>(0); | ||||||
|
||||||
function cancelRaf() { | ||||||
const cancelRaf = React.useCallback(function cancelRaf() { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 不需要包 useCallback,原本代码里也是不加到 deps 里的。这里加了下面放到 effect 的 deps 是没有必要的。 |
||||||
promiseIdRef.current += 1; | ||||||
} | ||||||
}, []); | ||||||
|
||||||
function collectHeight(sync = false) { | ||||||
const collectHeight = React.useCallback(function (sync = false) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. collectHeight 应该是没有作为过条件,也是不需要 useCallback 的 |
||||||
cancelRaf(); | ||||||
|
||||||
const doCollect = () => { | ||||||
|
@@ -67,9 +67,9 @@ export default function useHeights<T>( | |||||
} | ||||||
}); | ||||||
} | ||||||
} | ||||||
}, [cancelRaf]); | ||||||
|
||||||
function setInstanceRef(item: T, instance: HTMLElement) { | ||||||
const setInstanceRef = React.useCallback(function setInstanceRef(item: T, instance: HTMLElement) { | ||||||
const key = getKey(item); | ||||||
const origin = instanceRef.current.get(key); | ||||||
|
||||||
|
@@ -88,11 +88,12 @@ export default function useHeights<T>( | |||||
onItemRemove?.(item); | ||||||
} | ||||||
} | ||||||
} | ||||||
}, [collectHeight, getKey, onItemAdd, onItemRemove]); | ||||||
|
||||||
useEffect(() => { | ||||||
return cancelRaf; | ||||||
}, []); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The useEffect cleanup function should include
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
|
||||||
// This is somewhat confusing: when heightsRef.current.set is called, updatedMark changes, which in turn causes heightsRef.current to also change. | ||||||
return [setInstanceRef, collectHeight, heightsRef.current, updatedMark]; | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里不用改,ResizeObserver 已经做过隔离了:
https://github.com/react-component/resize-observer/blob/a6c5f48c43a8de415899ddc1a693406dcf9aee90/src/SingleObserver/index.tsx#L49