-
Notifications
You must be signed in to change notification settings - Fork 586
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
refactor(Upload): convert to TypeScript, impove docs and tests, close #4622 #4841
refactor(Upload): convert to TypeScript, impove docs and tests, close #4622 #4841
Conversation
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.
这是您为 Fusion/Next 提的第一个 pr,感谢您为 Fusion 做出的贡献,我们会尽快进行处理。
6e7820e
to
4abb28c
Compare
重新提交了一版 |
重新打开PR |
@@ -194,7 +169,7 @@ class Card extends Base { | |||
listType="card" | |||
closable | |||
locale={locale} | |||
value={this.state.value} | |||
value={this.state.value as UploadFile[]} |
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.
state.value 本身不就是 UploadFile[] 吗,这里为什么还需要 as
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.
这个还没有改
提交了一个commit,修复了上面的问题 |
@@ -194,7 +169,7 @@ class Card extends Base { | |||
listType="card" | |||
closable | |||
locale={locale} | |||
value={this.state.value} | |||
value={this.state.value as UploadFile[]} |
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.
这个还没有改
const prefixCls = `${this.props.prefix}upload`; | ||
const downloadURL = file.downloadURL || file.url; | ||
const imgURL = file.imgURL || file.url; | ||
const size = this.sizeCaculator(file.size); | ||
const size = this.sizeCaculator(file.size as unknown as string); |
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.
这里还有 as unknown
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.
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.
那这里应该用 expect-error 并说明原因,不要出现 as unknown。
@@ -190,16 +149,25 @@ class List extends Component { | |||
} | |||
|
|||
const suffix = SIZE_SUFFIX[suffixIndex]; | |||
fileSize = fileSize.toFixed(2); | |||
fileSize = fileSize.toFixed(2) as unknown as number; |
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.
同上
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.
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.
同上
@@ -618,7 +495,7 @@ class Upload extends Base { | |||
actionRender={actionRender} | |||
uploader={this} | |||
listType={listType} | |||
value={this.state.value} | |||
value={this.state.value as UploadFile[]} |
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.
这里也有类似问题
export function removeFileItem<T extends { uid?: string | number; name?: string }>( | ||
file: T, | ||
fileList: T[] | ||
) { |
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.
这里感觉用泛型把类型收的太窄了。从实现上来看,这个 remove 逻辑,并不要求 fileList 的 item 必须要和 file 同型,只是过滤 uid 或者 name 相等的情况。即使其他字段不相同,也是可以过滤的。而这里加了泛型之后,就要求两者类型必须完全一致了。这也是为什么下面测试里还得额外指定类型的原因了。
}); | ||
it('removeFileItem not find one to remove', () => { | ||
const file = { uid: 1, 1: 1 }; | ||
const files = [{ uid: 3, 3: 3 }, { uid: 2, 2: 2 }]; | ||
const files: { uid: number; [key: number]: number }[] = [ |
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.
这里没必要指定类型,removeFileItem 那里已经做出了说明。
const prefixCls = `${this.props.prefix}upload`; | ||
const downloadURL = file.downloadURL || file.url; | ||
const imgURL = file.imgURL || file.url; | ||
const size = this.sizeCaculator(file.size); | ||
const size = this.sizeCaculator(file.size as unknown as string); |
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.
那这里应该用 expect-error 并说明原因,不要出现 as unknown。
000aa10
into
alibaba-fusion:refactor/upload
close #4622