Skip to content

refactor(pagination): data abstraction via hooks - #2988

Merged
xiaoyatong merged 5 commits into
jdf2e:feat_v3.xfrom
oasis-cloud:feat_pagination_hooks
Mar 12, 2025
Merged

xiaoyatong merged 5 commits into
jdf2e:feat_v3.xfrom
oasis-cloud:feat_pagination_hooks

Conversation

@oasis-cloud

@oasis-cloud oasis-cloud commented Feb 13, 2025

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • 新功能
    • 引入全新分页工具,改善了分页按钮的生成和显示,提升翻页及导航体验。
  • 重构
    • 精简了分页组件的状态管理和事件处理,使页面翻页更加直观流畅。

@coderabbitai

coderabbitai Bot commented Feb 13, 2025

Copy link
Copy Markdown

Walkthrough

该提交引入了一个新的分页钩子(usePagination),用于集中管理分页相关的逻辑。新的钩子在 src/hooks/use-pagination.ts 中定义了接口、类型和辅助函数,用于计算分页按钮与总分页数。同时,在两个分页组件(src/packages/pagination/pagination.taro.tsxsrc/packages/pagination/pagination.tsx)中,原有的分页逻辑被替换为对该钩子的调用,并对状态变量和事件处理函数进行了重命名和优化。

Changes

文件 更改说明
src/hooks/use-pagination.ts 新增分页钩子,定义了 PaginationOptions 接口、PaginationNodePaginationResult 类型;添加了 human2MachinecalculateNodesaddEllipses 函数及 usePagination hook。
src/packages/pagination/pagination.taro.tsx
src/packages/pagination/pagination.tsx
在分页组件中移除了原有分页计算逻辑,使用 usePagination 钩子替换;重命名状态变量(如 currentPagecurrent),并更新了点击事件函数(如 handleSelectPagehandleClick)、前后页切换函数(如 prevPagenextPage)。

Sequence Diagram(s)

sequenceDiagram
    participant User as 用户
    participant PC as 分页组件
    participant UP as usePagination 钩子

    User->>PC: 点击分页按钮或前/后翻页按钮
    PC->>UP: 调用 usePagination(total, current, ...options)
    UP-->>PC: 返回 pages 与 pageCount
    PC->>PC: 更新状态(current)并重新渲染分页组件
Loading

Poem

我是一只跳跃的小兔子,
欣喜看到代码整装待发,
分页钩子悄然上线,
逻辑简化,状态划一如画,
点点按钮跳动乐声响,
我心随代码一起欢跃! 🐰✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

 ERR_PNPM_OPTIONAL_DEPS_REQUIRE_PROD_DEPS  Optional dependencies cannot be installed without production dependencies

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions Bot added action:review This PR needs more reviews (less than 2 approvals) 3.x Target branch 3.x labels Feb 13, 2025
@codecov

codecov Bot commented Feb 13, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.76271% with 5 lines in your changes missing coverage. Please review.

Project coverage is 86.15%. Comparing base (a73cf6b) to head (1ce75a2).
Report is 2 commits behind head on feat_v3.x.

Files with missing lines Patch % Lines
src/hooks/use-pagination.ts 94.80% 4 Missing ⚠️
src/packages/pagination/pagination.tsx 97.56% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           feat_v3.x    #2988      +/-   ##
=============================================
+ Coverage      86.10%   86.15%   +0.04%     
=============================================
  Files            277      278       +1     
  Lines          18408    18470      +62     
  Branches        2807     2814       +7     
=============================================
+ Hits           15851    15912      +61     
- Misses          2552     2553       +1     
  Partials           5        5              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/packages/pagination/pagination.tsx (1)

142-142: 静态分析提示测试覆盖率不足。
第 142 行涉及渲染处理,可能在测试中未覆盖 mode="lite" 或者特殊场景。可添加对这部分渲染和点击交互的测试用例,以提高整体测试覆盖率。

需要帮助编写测试用例吗?我可以提供一个基础测试方案来确保此代码路径得到充分验证。

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests

src/packages/pagination/pagination.taro.tsx (1)

95-99: 前一页按钮的可用状态样式逻辑清晰,但可考虑辅助文本提示。
建议在禁用时加上相应的 aria-disabled 等属性以增强可访问性。

<View
   className={classNames({
     ...
   })}
+  aria-disabled={current === 1}
   onClick={() => prevPage()}
>
src/hooks/use-pagination.ts (2)

30-32: human2Machine 函数命名直观,但使用自减运算符可能引发误解。
减少一行后再返回,容易让读者对返回值及副作用判断不清,可将 number - 1 直接写在 return 中以提升可读性。

-function human2Machine(number: number) {
-  return --number
+function human2Machine(number: number) {
+  return number - 1
 }

52-53: 静态分析提示分支场景的测试覆盖率不足。
calculateButtons 内部,当尾部触达极限或首部边界时,会触发不同的 start/end 调整逻辑;静态分析的警告显示这部分可能缺乏测试用例。可通过添加极端场景(大分页数、小分页数等)进行验证,确保所有分支能正确执行。

是否需要我为这些分支逻辑提供示例测试代码,以便及时补充测试覆盖率?

Also applies to: 55-56

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 52-53: src/hooks/use-pagination.ts#L52-L53
Added lines #L52 - L53 were not covered by tests

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7de3afb and e3c44fa.

📒 Files selected for processing (6)
  • packages/nutui-taro-demo/config/index.js (1 hunks)
  • packages/nutui-taro-demo/tsconfig.json (1 hunks)
  • src/hooks/use-pagination.ts (1 hunks)
  • src/packages/pagination/pagination.taro.tsx (4 hunks)
  • src/packages/pagination/pagination.tsx (4 hunks)
  • tsconfig.h5.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • tsconfig.h5.json
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pagination/pagination.tsx

[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests

src/hooks/use-pagination.ts

[warning] 52-53: src/hooks/use-pagination.ts#L52-L53
Added lines #L52 - L53 were not covered by tests


[warning] 55-56: src/hooks/use-pagination.ts#L55-L56
Added lines #L55 - L56 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test
🔇 Additional comments (9)
src/packages/pagination/pagination.tsx (3)

59-64: usePropsValue 中使用 current 状态没有问题,命名清晰且与组件语义相符。
此处逻辑简单明了,能够有效同步外部 value 与内部状态,不存在明显问题。


74-78: 点击事件的边界检查非常必要,避免了无效操作。
对于超出范围或已经选中的页码进行了短路处理,可读性和安全性都较好。


125-125: 在简单模式下显示当前页码与总页数,能保持简洁,但仍需注意用例覆盖。
建议在测试中包含 mode="simple" 的场景,确保 currentpageCount 的计算与渲染在极端值时也能正确工作。

src/packages/pagination/pagination.taro.tsx (2)

76-80: 点击事件边界校验合理,可避免无效或重复操作。
与普通版组件的要点保持一致,防止用户在边界外翻页导致状态异常。


142-143: 与行 142-143 相关的颜色设置需完善测试覆盖率。
同样地,如果在某些模式下 next 按钮在边界不可用,需测试并验证 addColorForHarmony 对配色的处理是否符合预期。

src/hooks/use-pagination.ts (1)

76-97: “省略符号”逻辑设计良好,可读性高。
在项目需求中常见此模式,可灵活隐藏中间页码。整体实现足以应对大部分分页场景,无需额外改动。

packages/nutui-taro-demo/config/index.js (1)

123-123: 新增 hooks 别名配置正确!

新增的 @/hooks 别名配置遵循了项目现有的别名命名规范,路径解析也正确指向了 hooks 目录。这个改动很好地支持了通过 hooks 实现数据抽象的目标。

packages/nutui-taro-demo/tsconfig.json (2)

34-36: TypeScript 路径映射配置完善!

新增的 @/hooks 路径映射与现有配置保持一致,正确使用了通配符模式。这个配置确保了 TypeScript 能够正确解析 hooks 模块的导入。


123-123:

Details

✅ Verification successful

验证 hooks 配置的一致性

建议验证 hooks 目录结构是否已经建立,确保配置能够正常工作。

Also applies to: 34-36


🏁 Script executed:

#!/bin/bash
# 验证 hooks 目录是否存在且包含文件
test -d src/hooks && find src/hooks -type f -name "*.ts" -o -name "*.tsx"

Length of output: 102


验证 hooks 配置一致性

经过验证,项目中 src/hooks/ 目录已存在,并且包含诸如 src/hooks/use-pagination.ts 文件,证明 hooks 配置在各文件中是一致且正确的。同时,其他提到的配置(如 34-36 行)也保持了相应的一致性。请确保所有 hooks 文件维持该配置要求。

@github-actions github-actions Bot added action:review This PR needs more reviews (less than 2 approvals) and removed action:review This PR needs more reviews (less than 2 approvals) labels Feb 13, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
src/packages/pagination/pagination.tsx (2)

74-86: 建议优化事件处理函数的结构!

通过使用提前返回(early return)模式可以使代码更清晰:

 const prevPage = () => {
-  const prev = current - 1
-  prev >= 1 && setCurrent(prev)
+  if (current <= 1) return
+  setCurrent(current - 1)
 }
 const nextPage = () => {
-  const next = current + 1
-  next <= pageCount && setCurrent(next)
+  if (current >= pageCount) return
+  setCurrent(current + 1)
 }

142-142: 需要补充测试用例!

lite 模式的渲染逻辑缺少测试覆盖。建议添加以下场景的测试:

  1. 验证当前页码显示
  2. 验证分隔符显示
  3. 验证总页数显示

需要我帮您生成测试用例吗?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests

src/packages/pagination/pagination.taro.tsx (2)

130-130: 可以简化字符串模板的使用!

-{`${current}/${pageCount}`}
+{current}/{pageCount}

当前的模板字符串用法过于复杂,可以简化为直接的 JSX 表达式。


1-160: 建议提取共享逻辑!

Web 版本和 Taro 版本的分页组件存在大量重复代码。建议:

  1. 创建一个共享的基础组件类,包含通用的分页逻辑
  2. Web 和 Taro 版本继承这个基础类,只处理平台特定的渲染逻辑
  3. 使用组合模式将通用的计算逻辑与平台特定的 UI 渲染分离

这样可以:

  • 减少代码重复
  • 简化维护工作
  • 确保跨平台行为一致性
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3c44fa and aec90f6.

📒 Files selected for processing (2)
  • src/packages/pagination/pagination.taro.tsx (4 hunks)
  • src/packages/pagination/pagination.tsx (4 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pagination/pagination.tsx

[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test
🔇 Additional comments (1)
src/packages/pagination/pagination.tsx (1)

1-74: 代码重构改进了关注点分离!

通过引入 usePagination 钩子将分页逻辑抽象到独立的模块中,提高了代码的可维护性和可重用性。同时使用 ButtonItem 类型增强了类型安全性。

Comment thread src/packages/pagination/pagination.taro.tsx
@oasis-cloud oasis-cloud changed the title refactor: data abstraction via hooks refactor(pagination): data abstraction via hooks Feb 17, 2025
Comment thread src/packages/pagination/pagination.tsx Outdated
className={classNames(
`${classPrefix}-next`,
currentPage >= pageCount ? `${classPrefix}-next-disabled` : ''
current >= pageCount ? `${classPrefix}-next-disabled` : ''

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

这里也改以下吧

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/hooks/use-pagination.ts Outdated
ellipse: boolean
}

export type ButtonItem = { number: number; text: string; selected?: boolean }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

听上去像个节点。。可以直接叫 PaginationOption

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

相关都可以改为 options 等~

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/hooks/use-pagination.ts Outdated
// 指示器
buttons: Array<ButtonItem>,
// 指示器的总数量
buttonsCount: number,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

直接 count 就可以了

# Conflicts:
#	src/packages/pagination/pagination.taro.tsx
#	src/packages/pagination/pagination.tsx
@oasis-cloud
oasis-cloud requested a review from xiaoyatong March 12, 2025 07:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (5)
src/hooks/use-pagination.ts (5)

1-12: 建议导出 PaginationOptions 接口

PaginationOptions 接口定义了分页参数,但它没有被导出。由于 usePagination 钩子接受这个接口作为参数,建议将其导出,使外部调用者能够更清晰地了解所需参数的类型。

-interface PaginationOptions {
+export interface PaginationOptions {
  // 当前页码, 从 1 开始
  current: number
  // 数据总条数
  total: number
  // 每页显示的条目数
  itemsPerPage: number
  // 指示器条目数量
  displayCount: number
  // 省略符号
  ellipse: boolean
}

20-25: 建议使用小写命名

PaginationResult 类型中的字段名称 PaginationNodesPaginationNodesCount 使用了大写开头,不符合 TypeScript/JavaScript 的常规变量命名规范。建议改为小写开头,以保持命名一致性。

type PaginationResult = [
  // 指示器
-  PaginationNodes: Array<PaginationNode>,
+  paginationNodes: Array<PaginationNode>,
  // 指示器的总数量
-  PaginationNodesCount: number,
+  paginationNodesCount: number,
]

34-36: 建议使用更具描述性的函数名

human2Machine 函数名称不够直观,难以理解其用途。建议使用更具描述性的名称,如 oneBasedToZeroBasedhumanIndexToMachineIndex

-function human2Machine(number: number) {
+function oneBasedToZeroBased(number: number) {
  return --number
}

80-101: 避免直接修改数组

addEllipses 函数直接修改了传入的 nodes 数组,这在 React 环境中可能导致不可预期的行为。建议返回一个新数组,避免副作用。

function addEllipses(
  nodes: Array<PaginationNode>,
  {
    displayCount,
    nodesCount,
    ellipse,
  }: { displayCount: number; nodesCount: number; ellipse: boolean }
) {
  if (nodesCount <= displayCount || !ellipse) return nodes
  const start = nodes[0]
  const end = nodes[nodes.length - 1]

  const leftEllipse = start.number > 1
  const rightEllipse = end.number < nodesCount
+  const result = [...nodes]
  if (leftEllipse) {
-    nodes.unshift({ number: start.number - 1, text: '...' })
+    result.unshift({ number: start.number - 1, text: '...' })
  }
  if (rightEllipse) {
-    nodes.push({ number: end.number + 1, text: '...' })
+    result.push({ number: end.number + 1, text: '...' })
  }
-  return nodes
+  return result
}

103-112: 添加输入验证

usePagination 钩子没有验证输入参数的有效性,例如 itemsPerPage 应该大于 0。建议添加适当的验证逻辑以提高代码的健壮性。

export const usePagination = (options: PaginationOptions): PaginationResult => {
  const mergedOptions = {
    ...defaultPaginationOptions,
    ...options,
  }
  const { total, itemsPerPage } = mergedOptions
+  
+  if (itemsPerPage <= 0) {
+    throw new Error('itemsPerPage must be greater than 0')
+  }
+  
  const nodesCount = Math.ceil((total || 0) / itemsPerPage) || 1

  return [calculateNodes(mergedOptions, nodesCount), nodesCount]
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aec90f6 and 1ce75a2.

📒 Files selected for processing (1)
  • src/hooks/use-pagination.ts (1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/hooks/use-pagination.ts

[warning] 56-57: src/hooks/use-pagination.ts#L56-L57
Added lines #L56 - L57 were not covered by tests


[warning] 59-60: src/hooks/use-pagination.ts#L59-L60
Added lines #L59 - L60 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test

Comment on lines +27 to +32
const defaultPaginationOptions: Partial<PaginationOptions> = {
current: 0,
itemsPerPage: 10,
displayCount: 5,
ellipse: false,
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

修正默认值与接口定义的不一致

根据接口注释,current 应该从 1 开始计数,但默认值却设为了 0。这与接口定义不符,可能导致计算错误。

const defaultPaginationOptions: Partial<PaginationOptions> = {
-  current: 0,
+  current: 1,
  itemsPerPage: 10,
  displayCount: 5,
  ellipse: false,
}
📝 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 defaultPaginationOptions: Partial<PaginationOptions> = {
current: 0,
itemsPerPage: 10,
displayCount: 5,
ellipse: false,
}
const defaultPaginationOptions: Partial<PaginationOptions> = {
current: 1,
itemsPerPage: 10,
displayCount: 5,
ellipse: false,
}

Comment on lines +38 to +78
function calculateNodes(options: PaginationOptions, nodesCount: number) {
// 分页器内部的索引从 0 开始,用户使用的索引从 1 开始
const halfIndex = Math.floor(options.displayCount / 2)
const buttonsCountIndex = human2Machine(nodesCount)
const displayCountIndex = human2Machine(options.displayCount)
const currentIndex = human2Machine(options.current)
let start
let end
if (buttonsCountIndex <= displayCountIndex) {
start = 0
end = buttonsCountIndex
} else {
start = Math.max(0, currentIndex - halfIndex)
end = Math.min(buttonsCountIndex, currentIndex + halfIndex)
if (end - start < displayCountIndex) {
if (start === 0) {
end = Math.min(start + displayCountIndex, buttonsCountIndex)
} else if (end === buttonsCountIndex) {
start = Math.max(end - displayCountIndex, 1)
}
} else if (end - start > displayCountIndex) {
end = start + displayCountIndex
}
}

const buttons = []
for (let i = start; i <= end; i++) {
const humanPageNumber = i + 1
buttons.push({
number: humanPageNumber,
text: humanPageNumber.toString(),
selected: options.current === humanPageNumber,
})
}

return addEllipses(buttons, {
nodesCount,
ellipse: options.ellipse,
displayCount: options.displayCount,
})
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

优化 calculateNodes 函数

calculateNodes 函数有几个需要注意的地方:

  1. 函数内部调用 human2Machine 的地方应更新为新的函数名
  2. 第 56 行的 Math.max(end - displayCountIndex, 1) 中,最小值应为 0 而非 1,因为索引是从 0 开始的
  3. 缺少对 options.displayCount 为 0 或负数的验证,可能导致计算错误

同时,根据静态分析,第 56-57 行和 59-60 行代码未被测试覆盖,建议增加相应测试用例。

function calculateNodes(options: PaginationOptions, nodesCount: number) {
  // 分页器内部的索引从 0 开始,用户使用的索引从 1 开始
  const halfIndex = Math.floor(options.displayCount / 2)
-  const buttonsCountIndex = human2Machine(nodesCount)
-  const displayCountIndex = human2Machine(options.displayCount)
-  const currentIndex = human2Machine(options.current)
+  const buttonsCountIndex = oneBasedToZeroBased(nodesCount)
+  const displayCountIndex = oneBasedToZeroBased(options.displayCount)
+  const currentIndex = oneBasedToZeroBased(options.current)

+  if (options.displayCount <= 0) {
+    throw new Error('displayCount must be greater than 0')
+  }

  let start
  let end
  if (buttonsCountIndex <= displayCountIndex) {
    start = 0
    end = buttonsCountIndex
  } else {
    start = Math.max(0, currentIndex - halfIndex)
    end = Math.min(buttonsCountIndex, currentIndex + halfIndex)
    if (end - start < displayCountIndex) {
      if (start === 0) {
        end = Math.min(start + displayCountIndex, buttonsCountIndex)
      } else if (end === buttonsCountIndex) {
-        start = Math.max(end - displayCountIndex, 1)
+        start = Math.max(end - displayCountIndex, 0)
      }
    } else if (end - start > displayCountIndex) {
      end = start + displayCountIndex
    }
  }

  const buttons = []
  for (let i = start; i <= end; i++) {
    const humanPageNumber = i + 1
    buttons.push({
      number: humanPageNumber,
      text: humanPageNumber.toString(),
      selected: options.current === humanPageNumber,
    })
  }

  return addEllipses(buttons, {
    nodesCount,
    ellipse: options.ellipse,
    displayCount: options.displayCount,
  })
}

请运行以下脚本验证未覆盖的代码路径:


🏁 Script executed:

#!/bin/bash
echo "检查未覆盖的代码路径..."
echo "查找测试文件:"
fd -t f --exec grep -l "usePagination" {} \; | grep test
echo "查找测试覆盖率报告:"
fd -t f -e json coverage

Length of output: 457


修正 calculateNodes 函数调用和边界验证问题

  • 请将函数内部调用 human2Machine 替换为 oneBasedToZeroBased 以保持一致;
  • 对于第 56 行的调整,修改 Math.max(end - displayCountIndex, 1)Math.max(end - displayCountIndex, 0),确保索引从 0 开始;
  • 增加对 options.displayCount 参数的有效性验证(仅允许正数),防止出现计算错误;
  • 注意:当前代码中第 56-57 行以及第 59-60 行的逻辑没有对应测试覆盖,请补充相应的单元测试确保这部分逻辑的正确性。

Diff 供参考:

function calculateNodes(options: PaginationOptions, nodesCount: number) {
  // 分页器内部的索引从 0 开始,用户使用的索引从 1 开始
-  const buttonsCountIndex = human2Machine(nodesCount)
-  const displayCountIndex = human2Machine(options.displayCount)
-  const currentIndex = human2Machine(options.current)
+  const buttonsCountIndex = oneBasedToZeroBased(nodesCount)
+  const displayCountIndex = oneBasedToZeroBased(options.displayCount)
+  const currentIndex = oneBasedToZeroBased(options.current)

+  if (options.displayCount <= 0) {
+    throw new Error('displayCount must be greater than 0')
+  }

  let start
  let end
  if (buttonsCountIndex <= displayCountIndex) {
    start = 0
    end = buttonsCountIndex
  } else {
    start = Math.max(0, currentIndex - halfIndex)
    end = Math.min(buttonsCountIndex, currentIndex + halfIndex)
    if (end - start < displayCountIndex) {
      if (start === 0) {
        end = Math.min(start + displayCountIndex, buttonsCountIndex)
      } else if (end === buttonsCountIndex) {
-        start = Math.max(end - displayCountIndex, 1)
+        start = Math.max(end - displayCountIndex, 0)
      }
    } else if (end - start > displayCountIndex) {
      end = start + displayCountIndex
    }
  }
  // ... 省略后续代码
}
📝 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
function calculateNodes(options: PaginationOptions, nodesCount: number) {
// 分页器内部的索引从 0 开始,用户使用的索引从 1 开始
const halfIndex = Math.floor(options.displayCount / 2)
const buttonsCountIndex = human2Machine(nodesCount)
const displayCountIndex = human2Machine(options.displayCount)
const currentIndex = human2Machine(options.current)
let start
let end
if (buttonsCountIndex <= displayCountIndex) {
start = 0
end = buttonsCountIndex
} else {
start = Math.max(0, currentIndex - halfIndex)
end = Math.min(buttonsCountIndex, currentIndex + halfIndex)
if (end - start < displayCountIndex) {
if (start === 0) {
end = Math.min(start + displayCountIndex, buttonsCountIndex)
} else if (end === buttonsCountIndex) {
start = Math.max(end - displayCountIndex, 1)
}
} else if (end - start > displayCountIndex) {
end = start + displayCountIndex
}
}
const buttons = []
for (let i = start; i <= end; i++) {
const humanPageNumber = i + 1
buttons.push({
number: humanPageNumber,
text: humanPageNumber.toString(),
selected: options.current === humanPageNumber,
})
}
return addEllipses(buttons, {
nodesCount,
ellipse: options.ellipse,
displayCount: options.displayCount,
})
}
function calculateNodes(options: PaginationOptions, nodesCount: number) {
// 分页器内部的索引从 0 开始,用户使用的索引从 1 开始
const halfIndex = Math.floor(options.displayCount / 2)
const buttonsCountIndex = oneBasedToZeroBased(nodesCount)
const displayCountIndex = oneBasedToZeroBased(options.displayCount)
const currentIndex = oneBasedToZeroBased(options.current)
if (options.displayCount <= 0) {
throw new Error('displayCount must be greater than 0')
}
let start
let end
if (buttonsCountIndex <= displayCountIndex) {
start = 0
end = buttonsCountIndex
} else {
start = Math.max(0, currentIndex - halfIndex)
end = Math.min(buttonsCountIndex, currentIndex + halfIndex)
if (end - start < displayCountIndex) {
if (start === 0) {
end = Math.min(start + displayCountIndex, buttonsCountIndex)
} else if (end === buttonsCountIndex) {
start = Math.max(end - displayCountIndex, 0)
}
} else if (end - start > displayCountIndex) {
end = start + displayCountIndex
}
}
const buttons = []
for (let i = start; i <= end; i++) {
const humanPageNumber = i + 1
buttons.push({
number: humanPageNumber,
text: humanPageNumber.toString(),
selected: options.current === humanPageNumber,
})
}
return addEllipses(buttons, {
nodesCount,
ellipse: options.ellipse,
displayCount: options.displayCount,
})
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 56-57: src/hooks/use-pagination.ts#L56-L57
Added lines #L56 - L57 were not covered by tests


[warning] 59-60: src/hooks/use-pagination.ts#L59-L60
Added lines #L59 - L60 were not covered by tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.x Target branch 3.x action:review This PR needs more reviews (less than 2 approvals) size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants