Fix some todos #289
Fix some todos #289
Conversation
| @@ -40,10 +42,16 @@ export class File { | |||
| })) | |||
| } | |||
|
|
|||
| // TODO: remember to do URL.revokeObjectURL | |||
| async url() { | |||
| async url(onCleanup: (disposer: Disposer) => void) { | |||
There was a problem hiding this comment.
考虑不要暴露url()，让用户使用URL.createObjectURL自己创建和销毁，可以用useFileUrl(file): url包装watchEffect来监听文件改动？另外，Blob中的type是必须的，之前遇到过音频无法播放/下载扩展名错误的问题。
There was a problem hiding this comment.
考虑不要暴露url()，让用户使用URL.createObjectURL自己创建和销毁
这个是考虑将来 url() 有可能不返回 object url，而是返回 file 本身的 public url（file loaded from public url 可以视作一种特殊的 lazy file，它知道文件内容对应的 public url），所以把获得 URL 的逻辑做在 File 上
另外，Blob中的type是必须的，之前遇到过音频无法播放/下载扩展名错误的问题。
这个没太理解，这里是希望 url() 返回 type 信息？
手动包装一个Disposer有些多余
“包装一个 Disposer”是指？
There was a problem hiding this comment.
这个没太理解，这里是希望
url()返回 type 信息？
噢我明白你意思了，是指 URL.createObjectURL(new Blob([ab])) 构造 blob 的时候要传入 type？
There was a problem hiding this comment.
这个是考虑将来 url() 有可能不返回 object url，而是返回 file 本身的 public url
如果要有这个逻辑应该分开？因为 public url不需要 revoke 而从blob创建的url需要。
噢我明白你意思了，是指
URL.createObjectURL(new Blob([ab]))构造 blob 的时候要传入 type？
对的
“包装一个 Disposer”是指？
是指创建 URL 时通过传入 onCleanup 来清除 URL，如果按我说的由外部来创建/清除就不需要这样传入onCleanup的逻辑。我觉得传入onCleanup这个行为有些复杂/不别要
There was a problem hiding this comment.
只是“声明一份数据使用的生命周期”，这个好像没有那么简洁的形式
将来借助 https://github.com/tc39/proposal-explicit-resource-management 这个也是一个可能的思路，它对使用者来说是简洁的；不过那个东西我没怎么用过还，跟 vue/react 结合使用会不会有坑还不清楚
There was a problem hiding this comment.
看了下getSize这个方法，应该还是可以在resolve之后手动调用revoke清理url，和这个比较像：
const playAudio = (asset: ExportedScratchFile) => {
const blob = new Blob([asset.arrayBuffer], { type: getMimeFromExt(asset.extension) })
const url = URL.createObjectURL(blob)
let audio = new Audio(url)
audio.play()
audio.onended = () => {
URL.revokeObjectURL(url)
}
}要求传入 onCleanup 就是在要求使用方声明 url 使用的生命周期
如果需要使用方显式地声明生命周期，这个接口从复杂度来说可能和和不封装也一样。我个人更偏向“谁创建谁销毁”这种接口形式。
There was a problem hiding this comment.
看了下getSize这个方法，应该还是可以在resolve之后手动调用revoke清理url
当然可以由 getSize 去做，但是就像上面提到的，
外部就需要关心 createObjectURL & revokeObjectURL——后者很容易忘记，而
onCleanup是必传的参数，可以确保不会忘记
“确保没有内存泄漏”会比接口简单更重要
这个接口从复杂度来说可能和和不封装也一样
我做在 File url() 中不是希望屏蔽 create & revoke Objecet URL 的复杂度，而是希望屏蔽 object url 跟 public url 的差异
我个人更偏向“谁创建谁销毁”这种接口形式。
File url() 去 create Object URL，也由 File url() 去 revoke，这不也符合“谁创建谁销毁”吗？
There was a problem hiding this comment.
File url() 去 create Object URL，也由 File url() 去 revoke，这不也符合“谁创建谁销毁”吗？
销毁的时机是又用户来决定而不是 File.url() 来决定的
而是希望屏蔽 object url 跟 public url 的差异
public url 如果也需要传入一个 cleanup 参数我认为是不太合理的
后者很容易忘记
这个应该就是 CR 需要解决的问题了
There was a problem hiding this comment.
销毁的时机是又用户来决定而不是 File.url() 来决定的
对，用户调用 File.url() 并使用其结果，所以用户决定 File.url() cleanup 的时机，这不还是“谁创建谁销毁”吗？
public url 如果也需要传入一个 cleanup 参数我认为是不太合理的
不是 public url 需要传入 clean up，而是一份可能是 public url 的数据需要 clean up
这个应该就是 CR 需要解决的问题了
这样的问题依赖 CR 去发现才是不得已的，能让工具帮助检查当然更好
|
The PR environment is ready, please check the PR environment [Attention]: This environment will be automatically cleaned up after a certain period of time., please make sure to test it in time. If you have any questions, please contact the builder team. |
| @@ -49,7 +49,7 @@ export class File { | |||
| }) | |||
| const ab = await this.arrayBuffer() | |||
| if (cancelled) throw new Cancelled() | |||
| const url = URL.createObjectURL(new Blob([ab])) | |||
| const url = URL.createObjectURL(new Blob([ab], { type: this.type })) | |||
There was a problem hiding this comment.
这里的this.type我记得不会保存（保存到cloud之后再加载下来type信息会丢失）
There was a problem hiding this comment.
嗯现在调用上传接口的时候没有把 mime type 提交，这个我单独 PR 解决下吧，得去看下对象存储的接口
Hooks for File
Hooks to help consume URL of class
File, taking care of:Disable eslint rule
no-redeclareIt is already disabled for
.tsfiles, while not disabled for.vuefiles. Related:@vue/eslint-config-typescriptextendsplugin:@typescript-eslint/eslint-recommended: codeplugin:@typescript-eslint/eslint-recommendeddisablesno-redeclare(and other rules which are not needed for Typescript code): code@vue/eslint-config-typescriptdidn't disableno-redeclare: codeWe may need to disable more such rules manually later. Before that we better first check if
@vue/eslint-config-typescripthas fixed it. Related issue: vuejs/eslint-config-typescript#18Timeout for api client
Support timeout for request, with default timeout: 10s
Keep consistent order for sprites & sounds
So that each time project is saved or reloaded, the order for sprites & sounds keeps stable.
Others
validateNameutils/path>filename&stripExt