Skip to content

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.

Sign up for GitHub

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

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

perf: skipping re-rendering #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6c22ff7
perf: skipping re-rendering
muzea Aug 19, 2025
e400d64
fix: some test error
muzea Aug 20, 2025
ffaa854
test: no unnecessary re-render
muzea Aug 30, 2025
0dd37dd
fix: code style
muzea Aug 30, 2025
5c2d75b
fix: test code error
muzea Aug 30, 2025
9225e75
fix: remove sharedConfig
muzea Sep 15, 2025
205e410
test: origin scroll
muzea Sep 15, 2025
28fd6b2
docs: apply copilot suggestion
muzea Sep 15, 2025
462d96d
test: try to improve coverage
muzea Sep 25, 2025
169288c
Update tests/touch.test.js
muzea Sep 30, 2025
48c33b3
fix: 去掉没有必要的 useCallback
muzea Nov 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/Filler.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import ResizeObserver from 'rc-resize-observer';
import classNames from 'classnames';
import ResizeObserver from 'rc-resize-observer';
import * as React from 'react';

export type InnerProps = Pick<React.HTMLAttributes<HTMLDivElement>, 'role' | 'id'>;

Expand Down Expand Up @@ -69,15 +69,15 @@ const Filler = React.forwardRef(
};
}

const handleResize = ({ offsetHeight }) => {
if (offsetHeight && onInnerResize) {
onInnerResize();
}
};

return (
<div style={outerStyle}>
<ResizeObserver
onResize={({ offsetHeight }) => {
if (offsetHeight && onInnerResize) {
onInnerResize();
}
}}
>
<ResizeObserver onResize={handleResize}>
<div
style={innerStyle}
className={classNames({
Expand Down
8 changes: 2 additions & 6 deletions src/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -572,7 +568,7 @@ export function RawList<T>(props: ListProps<T>, ref: React.Ref<ListRef>) {
offsetLeft,
setInstanceRef,
children,
sharedConfig,
getKey,
Copy link

@coderabbitai coderabbitai bot Sep 15, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

useChildren 调用已改为传入 getKey；请检查仓库内是否仍有 SharedConfig 遗留引用

为避免类型/行为不一致，请全局确认：

  • 不再引用或构造 SharedConfig
  • 所有 useChildren 调用点参数顺序已更新

🏁 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 变更

  • src/Item.tsx：发现 refFunc 为
    const refFunc = React.useCallback(node => { setRef(node); }, []);
    缺少 setRef 依赖。修复为将依赖数组改为 [setRef]（或确保 setRef 为稳定引用）。位置：src/Item.tsx（refFunc 定义处）。
  • 未能确认仓库内是否仍有 SharedConfig 或未更新的 useChildren 调用（前两次 rg 搜索未返回结果）。全局确认：不再引用/构造 SharedConfig，且所有 useChildren 调用点均已改为传入 getKey 且参数顺序正确。可重跑校验命令：
    rg -nP --glob '!/node_modules/' '\bSharedConfig<'
    rg -nP --glob '!/node_modules/' 'useChildren\s*(' -C2
🤖 Prompt for AI Agents 
In src/List.tsx around line 571 and specifically in src/Item.tsx at the refFunc
definition, update the useCallback dependency so refFunc =
React.useCallback(node => { setRef(node); }, [setRef]) (or ensure setRef is a
stable ref/function) instead of an empty array; then run the two project's
checks to confirm no remaining SharedConfig usages and that all useChildren call
sites pass getKey with correct parameter order using: rg -nP --glob
'!**/node_modules/**' '\bSharedConfig<' and rg -nP --glob '!**/node_modules/**'
'useChildren\s*\(' -C2 and fix any findings accordingly.

);

let componentStyle: React.CSSProperties = null;
Expand Down
37 changes: 20 additions & 17 deletions src/hooks/useChildren.tsx
View file
Open in desktop
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>(
Expand All @@ -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
Copy link

@coderabbitai coderabbitai bot Sep 15, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

setRef 闭包与 Item 中 useCallback([]) 组合可能导致使用过期的 setRef

此处每次渲染都会生成新的 setRef 闭包。src/Item.tsx 中 refFunc 通过 useCallback 且依赖数组为空，会捕获首次的 setRef 并在后续保持不变，导致卸载/替换时可能调用过期闭包。请在 Item.tsx 中将依赖改为 [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,
   });
 }

🏁 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 为内联函数 (ele) => setNodeRef(item, ele)，会变化，导致卸载/替换时调用过期闭包。将依赖改为 [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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const key = getKey(item);
return (
<Item key={key} setRef={(ele) => setNodeRef(item, ele)}>
{node}
</Item>
);
export function Item({ children, setRef }: ItemProps) {
const refFunc = React.useCallback((node) => {
setRef(node);
}, [setRef]);
return React.cloneElement(children, {
ref: refFunc,
});
}
🤖 Prompt for AI Agents 
In src/Item.tsx around the useCallback that builds the ref callback (the current
implementation uses an empty dependency array), the ref closure captures a stale
setRef; update the useCallback dependencies to [setRef] so the ref function
always uses the latest setRef passed from useChildren (preserve the same ref
function signature and behavior, only change the dependency array).

});
}, [list, startIndex, endIndex, setNodeRef, renderFunc, getKey, offsetX, scrollWidth]);
Copy link
Member

@zombieJ zombieJ Sep 30, 2025

Choose a reason for hiding this comment

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

稍微有点复杂了，未来如果加 props 也容易 break。如果是只是为了 Item 不需要重新渲染，可以考虑 Item 直接用 React.memo 包一下，条件里直接忽略 setRef 即可。

}
5 changes: 3 additions & 2 deletions src/hooks/useHeights.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default function useHeights<T>(
}
}

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);

Expand All @@ -88,11 +88,12 @@ export default function useHeights<T>(
onItemRemove?.(item);
}
}
}
}, [getKey, onItemAdd, onItemRemove]);

useEffect(() => {
return cancelRaf;
}, []);
Copy link

Copilot AI Aug 19, 2025

Choose a reason for hiding this comment

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

The useEffect cleanup function should include cancelRaf in the dependency array since it's now a memoized callback. Change the dependency array from [] to [cancelRaf].

Suggested change
}, []);
}, [cancelRaf]);

Copilot uses AI. Check for mistakes.

// 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];
}
4 changes: 0 additions & 4 deletions src/interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ export type RenderFunc<T> = (
props: { style: React.CSSProperties; offsetX: number },
) => React.ReactNode;

export interface SharedConfig<T> {
getKey: (item: T) => React.Key;
}

export type GetKey<T> = (item: T) => React.Key;

export type GetSize = (startKey: React.Key, endKey?: React.Key) => { top: number; bottom: number };
Expand Down
57 changes: 37 additions & 20 deletions tests/props.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { mount } from 'enzyme';
import React from 'react';
import List from '../src';

describe('Props', () => {
Expand All @@ -11,30 +11,20 @@ describe('Props', () => {
}

const wrapper = mount(
<List data={[{ id: 903 }, { id: 1128 }]} itemKey={item => item.id}>
<List data={[{ id: 903 }, { id: 1128 }]} itemKey={(item) => item.id}>
{({ id }) => <ItemComponent>{id}</ItemComponent>}
</List>,
);

expect(
wrapper
.find('Item')
.at(0)
.key(),
).toBe('903');

expect(
wrapper
.find('Item')
.at(1)
.key(),
).toBe('1128');
expect(wrapper.find('Item').at(0).key()).toBe('903');

expect(wrapper.find('Item').at(1).key()).toBe('1128');
});

it('prefixCls', () => {
const wrapper = mount(
<List data={[0]} itemKey={id => id} prefixCls="prefix">
{id => <div>{id}</div>}
<List data={[0]} itemKey={(id) => id} prefixCls="prefix">
{(id) => <div>{id}</div>}
</List>,
);

Expand All @@ -44,13 +34,40 @@ describe('Props', () => {
it('offsetX in renderFn', () => {
let scrollLeft;
mount(
<List data={[0]} itemKey={id => id} prefixCls="prefix">
{(id, _, { offsetX }) => {
<List data={[0]} itemKey={(id) => id} prefixCls="prefix">
{(id, _, { offsetX }) => {
scrollLeft = offsetX;
return <div>{id}</div>}}
return <div>{id}</div>;
}}
</List>,
);

expect(scrollLeft).toEqual(0);
});

it('no unnecessary re-render', () => {
const renderItem = jest.fn();
renderItem.mockImplementation(({ id, key }) => <div key={key}>{id}</div>);

const data = [{ id: 1, key: 1 }];
function Wrapper() {
const [state, setState] = React.useState(0);

React.useEffect(() => {
setState(1);
}, []);

return (
<div>
<h1>{state}</h1>
<List data={data} itemKey="key" prefixCls="prefix">
{renderItem}
</List>
</div>
);
}
const wrapper = mount(<Wrapper />);
expect(wrapper.find('h1').text()).toBe('1');
expect(renderItem).toHaveBeenCalledTimes(1);
});
});
12 changes: 7 additions & 5 deletions tests/scroll-Firefox.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import React from 'react';
import { act } from 'react-dom/test-utils';
import { act } from '@testing-library/react';
import { mount } from 'enzyme';
import { spyElementPrototypes } from './utils/domHook';
import React from 'react';
import List from '../src';
import isFF from '../src/utils/isFirefox';
import { spyElementPrototypes } from './utils/domHook';

function genData(count) {
return new Array(count).fill(null).map((_, index) => ({ id: String(index) }));
Expand Down Expand Up @@ -124,8 +124,10 @@ describe('List.Firefox-Scroll', () => {
const wrapper = genList({ itemHeight: 20, height: 100, data: genData(100), ref: listRef });
const ulElement = wrapper.find('ul').instance();
// scroll to bottom
listRef.current.scrollTo(99999);
jest.runAllTimers();
act(() => {
listRef.current.scrollTo(99999);
jest.runAllTimers();
});
expect(wrapper.find('ul').instance().scrollTop).toEqual(1900);

act(() => {
Expand Down
31 changes: 21 additions & 10 deletions tests/scroll.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,13 @@ describe('List.Scroll', () => {
jest.useFakeTimers();
const listRef = React.createRef();
const wrapper = genList({ itemHeight: 20, height: 100, data: genData(100), ref: listRef });
jest.runAllTimers();

listRef.current.scrollTo(null);
act(() => {
jest.runAllTimers();

listRef.current.scrollTo(null);
});

expect(wrapper.find('.rc-virtual-list-scrollbar-thumb').props().style.display).not.toEqual(
'none',
);
Expand All @@ -107,8 +111,10 @@ describe('List.Scroll', () => {
it('value scroll', () => {
const listRef = React.createRef();
const wrapper = genList({ itemHeight: 20, height: 100, data: genData(100), ref: listRef });
listRef.current.scrollTo(903);
jest.runAllTimers();
act(() => {
listRef.current.scrollTo(903);
jest.runAllTimers();
});
expect(wrapper.find('ul').instance().scrollTop).toEqual(903);

wrapper.unmount();
Expand All @@ -125,9 +131,8 @@ describe('List.Scroll', () => {
...result,
ref,
scrollTo: (...args) => {
ref.current.scrollTo(...args);

act(() => {
ref.current.scrollTo(...args);
jest.runAllTimers();
});
},
Expand All @@ -153,8 +158,10 @@ describe('List.Scroll', () => {

it('scroll top should not out of range', () => {
const { scrollTo, container } = presetList();
scrollTo({ index: 0, align: 'bottom' });
jest.runAllTimers();
act(() => {
scrollTo({ index: 0, align: 'bottom' });
jest.runAllTimers();
});
expect(container.querySelector('ul').scrollTop).toEqual(0);
});

Expand Down Expand Up @@ -389,9 +396,13 @@ describe('List.Scroll', () => {
ref: listRef,
direction: 'rtl',
});
jest.runAllTimers();

listRef.current.scrollTo(null);
act(() => {
jest.runAllTimers();

listRef.current.scrollTo(null);
});

expect(wrapper.find('.rc-virtual-list-scrollbar-thumb').props().style.display).not.toEqual(
'none',
);
Expand Down
8 changes: 6 additions & 2 deletions tests/scrollWidth.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,14 @@ describe('List.scrollWidth', () => {
ref: listRef,
});

listRef.current.scrollTo({ left: 135 });
act(() => {
listRef.current.scrollTo({ left: 135 });
});
expect(listRef.current.getScrollInfo()).toEqual({ x: 135, y: 0 });

listRef.current.scrollTo({ left: -99 });
act(() => {
listRef.current.scrollTo({ left: -99 });
});
expect(listRef.current.getScrollInfo()).toEqual({ x: 0, y: 0 });
});

Expand Down
Loading