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(list): fix error reported due to incorrect paginator parameters #39681

Merged
merged 7 commits into from Dec 24, 2022

Conversation

Wxh16144
Copy link
Member

@Wxh16144 Wxh16144 commented Dec 20, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

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

🔗 Related issue link

fix #39496

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English Fix List crash when pagination.pageSize is undefined.
🇨🇳 Chinese 修复 List 组件分页器错误参数导致报错问题。

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • 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

@github-actions
Copy link
Contributor

github-actions bot commented Dec 20, 2022

@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (2689aff) compared to base (0e98cb5).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##            master    #39681   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          556       557    +1     
  Lines         9625      9626    +1     
  Branches      2722      2722           
=========================================
+ Hits          9625      9626    +1     
Impacted Files Coverage Δ
components/table/hooks/usePagination.ts 100.00% <ø> (ø)
components/_util/extendsObject.ts 100.00% <100.00%> (ø)
components/list/index.tsx 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -214,9 +236,9 @@ function List<T>({

let splitDataSource = [...dataSource];
if (pagination) {
if (dataSource.length > (paginationProps.current - 1) * paginationProps.pageSize) {
if (dataSource.length > (paginationProps.current! - 1) * paginationProps.pageSize!) {
Copy link
Contributor

Choose a reason for hiding this comment

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

! 这里有点不合适吧, 如果 current 是确保有的, 就用不上!, 如果可能没有, ! 就是掩盖了.

Copy link
Member Author

Choose a reason for hiding this comment

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

主要是 TS 类型推导会提示我错误, 原因是 extendsObject 的类型推导变了

Copy link
Contributor

Choose a reason for hiding this comment

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

我感觉这里不应该这样解, 因为类似的应用场景应该很普遍. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

你是指这个 bug 的解决方案应该调整一下么? 还是 ! 非空表达需要调整

Copy link
Contributor

Choose a reason for hiding this comment

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

我仍然认为这个应该由业务处理, antd不修复.

Copy link
Member Author

Choose a reason for hiding this comment

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

这个问题确实比较有争议,但是我们现在能做的应该和 Table 的行为保持一致,先挂在这里吧~

components/list/index.tsx Outdated Show resolved Hide resolved
/* eslint-disable no-redeclare */
export function mergeProps<A, B>(a: A, b: B): B & A;
export function mergeProps<A, B, C>(a: A, b: B, c: C): C & B & A;
export function mergeProps(...list: any[]) {
Copy link
Member

@MadCcc MadCcc Dec 22, 2022

Choose a reason for hiding this comment

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

建议直接把 extendObject 搬过来,或者就不用提了

Copy link
Member Author

Choose a reason for hiding this comment

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

原来的 extendObject 类型推导貌似没有这个强,这里我是参考了 antd-mobile 的 utils。

如果仍然需要调整,我可以直接把 extendObject 原封不动移动到 _utils/extendObject.ts

@afc163 afc163 merged commit 951e0e2 into ant-design:master Dec 24, 2022
@vagusX vagusX mentioned this pull request Dec 26, 2022
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.

List crash when pagination pageSize is undefined
4 participants