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

refactor(Upload): convert to TypeScript, impove docs and tests, close #4622 #4841

Merged
merged 2 commits into from
Jul 4, 2024

Conversation

FairyYang
Copy link
Collaborator

@FairyYang FairyYang commented May 13, 2024

close #4622

Copy link

@github-actions github-actions bot left a 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 做出的贡献,我们会尽快进行处理。

@eternalsky eternalsky self-requested a review May 14, 2024 02:03
@FairyYang FairyYang changed the title Upload typeScript refactor(Upload): convert to TypeScript, impove docs and tests, close #4622 May 14, 2024
components/upload/__docs__/demo/after-select/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/after-select/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/after-select/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/base/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/beforeupload/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/paste/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/paste/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/submit/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/submit/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/text/index.tsx Outdated Show resolved Hide resolved
@FairyYang FairyYang force-pushed the form-update branch 2 times, most recently from 6e7820e to 4abb28c Compare June 11, 2024 06:55
cypress/fixtures/742.png Outdated Show resolved Hide resolved
components/upload/__docs__/demo/after-select/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/crop/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/extra/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/extra/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/demo/submit/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/theme/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/theme/index.tsx Outdated Show resolved Hide resolved
components/upload/__docs__/theme/index.tsx Outdated Show resolved Hide resolved
components/upload/__tests__/card-spec.tsx Outdated Show resolved Hide resolved
components/upload/__tests__/card-spec.tsx Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
@FairyYang
Copy link
Collaborator Author

重新提交了一版

@FairyYang
Copy link
Collaborator Author

重新打开PR

components/upload/__docs__/demo/submit/index.tsx Outdated Show resolved Hide resolved
components/upload/__tests__/card-spec.tsx Outdated Show resolved Hide resolved
components/upload/__tests__/util-spec.ts Outdated Show resolved Hide resolved
components/upload/card.tsx Outdated Show resolved Hide resolved
@@ -194,7 +169,7 @@ class Card extends Base {
listType="card"
closable
locale={locale}
value={this.state.value}
value={this.state.value as UploadFile[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

state.value 本身不就是 UploadFile[] 吗,这里为什么还需要 as

Copy link
Contributor

Choose a reason for hiding this comment

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

这个还没有改

components/upload/runtime/html5-uploader.tsx Outdated Show resolved Hide resolved
components/upload/runtime/html5-uploader.tsx Outdated Show resolved Hide resolved
components/upload/runtime/request.tsx Outdated Show resolved Hide resolved
components/upload/runtime/request.tsx Outdated Show resolved Hide resolved
components/upload/util.ts Outdated Show resolved Hide resolved
@FairyYang
Copy link
Collaborator Author

提交了一个commit,修复了上面的问题

components/upload/__tests__/util-spec.ts Outdated Show resolved Hide resolved
@@ -194,7 +169,7 @@ class Card extends Base {
listType="card"
closable
locale={locale}
value={this.state.value}
value={this.state.value as UploadFile[]}
Copy link
Contributor

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

这里还有 as unknown

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

65E72D94-74D9-4350-8080-53586AF7902C

Copy link
Contributor

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;
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
Collaborator Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor

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[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

这里也有类似问题

components/upload/upload.tsx Outdated Show resolved Hide resolved
components/upload/upload.tsx Outdated Show resolved Hide resolved
components/upload/upload.tsx Outdated Show resolved Hide resolved
export function removeFileItem<T extends { uid?: string | number; name?: string }>(
file: T,
fileList: T[]
) {
Copy link
Contributor

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 }[] = [
Copy link
Contributor

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);
Copy link
Contributor

Choose a reason for hiding this comment

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

那这里应该用 expect-error 并说明原因,不要出现 as unknown。

@eternalsky eternalsky changed the base branch from master to refactor/upload July 4, 2024 02:27
@eternalsky eternalsky merged commit 000aa10 into alibaba-fusion:refactor/upload Jul 4, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

【Technical upgrade】Upload
2 participants