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: Delay folder upload until all children files are parsed #559

Closed
wants to merge 4 commits into from
Closed

fix: Delay folder upload until all children files are parsed #559

wants to merge 4 commits into from

Conversation

coderz-w
Copy link
Contributor

@coderz-w coderz-w commented Jun 7, 2024 •
edited by yoyo837
Loading

🤔 这个变动的性质是？

  • 新特性提交
  • 日常 bug 修复
  • 站点、文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • TypeScript 定义更新
  • 包体积优化
  • 性能优化
  • 功能增强
  • 国际化改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 工作流程
  • 其他改动（是关于什么的改动？）

🔗 相关 Issue

antd issue

💡 需求背景和解决方案

当拖拽上传文件夹时，需要手动解析文件树，原代码在解析到子文件为非文件夹时直接执行后续而非等待全部文件解析完成，与普通上传的表现不一致。添加其他变量确保文件全部解析后再进行后续操作

@vercel Vercel
Copy link

vercel bot commented Jun 7, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
upload ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2024 10:01am

afc163
afc163 reviewed Jun 7, 2024
View reviewed changes
src/traverseFileTree.ts Outdated
@@ -59,10 +61,15 @@ const traverseFileTree = (files: InternalDataTransferItem[], callback, isAccepte
},
});
}
callback([file]);
flattenFileList.push(file);
console.log(restDirectory);
Copy link
Member

Choose a reason for hiding this comment

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

不应该有这个。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

马上马上

@afc163
Copy link
Member

afc163 commented Jun 7, 2024

这个好写测试用例不

@coderz-w
Copy link
Contributor Author

coderz-w commented Jun 7, 2024 •
edited by yoyo837
Loading

这个好写测试用例不

其实根本不会写测试...,我先去看看吧，不行的话麻烦一下大佬了

@yoyo837
Copy link
Member

yoyo837 commented Jun 7, 2024

这个好写测试用例不

其实根本不会写测试...,我先去看看吧，不行的话麻烦一下大佬了

学，以后PR处处要写。写一次就会了。

@coderz-w
Copy link
Contributor Author

coderz-w commented Jun 7, 2024

这个好写测试用例不

其实根本不会写测试...,我先去看看吧，不行的话麻烦一下大佬了

学，以后PR处处要写。写一次就会了。

我尽量

@coderz-w
Copy link
Contributor Author

coderz-w commented Jun 8, 2024

@yoyo837 @afc163 佬，原有测试里模拟FileSystemEntry的函数处理文件夹时存在bug
1d587218108d49de87a2a2506271a1f2
，在测试 https://github.com/react-component/upload/blob/master/tests/uploader.spec.tsx#L377 将webp后缀改成png后测试任然通过可以验证这个bug。
我要加测试的话需要先把这个改了，我是直接在这里单独commit一次还是怎么弄呢

@coderz-w coderz-w mentioned this pull request Jun 10, 2024
Merged
@afc163
Copy link
Member

afc163 commented Jun 10, 2024

  1. 改动好大，能解释一下么
  2. ci 挂了看看

@coderz-w
Copy link
Contributor Author

  1. 改动好大，能解释一下么
  2. ci 挂了看看

就改了一行吧，return 那里，原因是应为
image在这个地方要通过传入参数长度判断文件是否处理完成，dirReader.readEntries里面传入的函数就是上面那张图里的handle，原来的写法会导致一直卡在loopFiles，后面的函数其实都没执行

@afc163
Copy link
Member

afc163 commented Jun 10, 2024

看 diff 不止一行改动。

@coderz-w
Copy link
Contributor Author

看 diff 不止一行改动。

测试那里我还没改，目前只是改了处理文件的地方，改动看起来比较大主要是应为把有个函数移到了另一个函数里面

@afc163
Copy link
Member

afc163 commented Jun 10, 2024

先做最小改动吧，重构可以延后或提前做掉。

@coderz-w
Copy link
Contributor Author

先做最小改动吧，重构可以延后或提前做掉。

我之前的改动太大了吗，要重新弄吗

@afc163
Copy link
Member

afc163 commented Jun 10, 2024

有个函数移到了另一个函数里面

这个是必须的改动么？

@coderz-w
Copy link
Contributor Author

有个函数移到了另一个函数里面

这个是必须的改动么？

需要有变量来记录文件处理情况，如果不这样搞那就在外层弄一个

@coderz-w coderz-w requested a review from afc163 June 18, 2024 00:19
@coderz-w coderz-w mentioned this pull request Jul 5, 2024
Merged
@coderz-w coderz-w closed this Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afc163 afc163 Awaiting requested review from afc163

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@coderz-w @afc163 @yoyo837