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.

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

Jump to bottom

fix(table): fix table shouldUpdate #2848

Merged
merged 3 commits into from
May 26, 2021
Merged

fix(table): fix table shouldUpdate #2848

merged 3 commits into from
May 26, 2021

Conversation

linxianxi
Copy link
Contributor

@linxianxi linxianxi commented May 26, 2021 •
edited
Loading

数组不能直接对比，导致直接在这一步全部 return 了。

@github-actions
Copy link

github-actions bot commented May 26, 2021 •
edited
Loading

⚡️ Deploying PR Preview...

DerrickTel
DerrickTel reviewed May 26, 2021
View reviewed changes
Copy link
Contributor

@DerrickTel DerrickTel left a comment

Choose a reason for hiding this comment

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

这种方法在某些情况下是可行的，当两个数组的元素顺序相同且元素都可以转换成字符串的情况下确实可行。
这样的代码存有隐患，比如数字被转换成字符串，数字“1”和字符串“1”会被认为相等，可能造成调试困难，不推荐使用。

@linxianxi
Copy link
Contributor Author

linxianxi commented May 26, 2021 •
edited
Loading

这种方法在某些情况下是可行的，当两个数组的元素顺序相同且元素都可以转换成字符串的情况下确实可行。
这样的代码存有隐患，比如数字被转换成字符串，数字“1”和字符串“1”会被认为相等，可能造成调试困难，不推荐使用。

还需要考虑顺序问题么，没仔细看，忘了 key 会有数字情况，以为都是字符串

@linxianxi
Copy link
Contributor Author

看了下，应该不需要判断顺序问题，比如当前 ["1,""2"]，去掉一个 “2” 已经 render 了，不可能一步直接到 ["2","1"]

@lgtm-com
Copy link
Contributor

lgtm-com bot commented May 26, 2021

This pull request introduces 1 alert when merging a0f1139 into 92093fa - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@codecov
Copy link

codecov bot commented May 26, 2021 •
edited
Loading

Codecov Report

Merging #2848 (a4078be) into master (92093fa) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2848       /-   ##
==========================================
- Coverage   99.07%   99.07%   -0.01%     
==========================================
  Files         173      173              
  Lines        5271     5269       -2     
  Branches     1883     1882       -1     
==========================================
- Hits         5222     5220       -2     
  Misses         48       48              
  Partials        1        1
Impacted Files Coverage Δ
packages/table/src/utils/genProColumnToColumn.tsx 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92093fa...a4078be. Read the comment docs.

@chenshuai2144 chenshuai2144 merged commit 6fa9928 into ant-design:master May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DerrickTel DerrickTel DerrickTel left review comments

@chenshuai2144 chenshuai2144 chenshuai2144 approved these changes

Assignees
No one assigned
Labels
PR: merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@linxianxi @chenshuai2144 @DerrickTel