feat: add AttachmentCard component with generic async upload lifecycle#142
feat: add AttachmentCard component with generic async upload lifecycle#142
Conversation
Introduce a reusable AttachmentCard API with image/file previews, menu actions, and optional async upload handling so products can implement upload transport externally while getting consistent loading and error UI. Made-with: Cursor
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| const temporaryItems = files.map(createTemporaryUploadItem); | ||
| setLocalItems((prev) => [...temporaryItems.map((entry) => entry.item), ...prev]); |
There was a problem hiding this comment.
[3/4 — Medium] Object URL leak on component unmount during active upload
createTemporaryUploadItem creates a blob: URL via URL.createObjectURL for image previews. The finally block in handleUpload correctly revokes it after resolution — but only if the Promise settles while the component is still mounted. If the component unmounts mid-upload (e.g., user navigates away), the async Promise.all continues running in the background, setLocalItems calls trigger React warnings, and critically the finally block still runs — but there's nothing preventing further issues from state updates on the unmounted component.
The larger risk is if uploadFile never settles (e.g., hung request) — the blob URL is held indefinitely.
Fix: add an isMounted guard or AbortController signal, and revoke all pending object URLs on cleanup:
React.useEffect(() => {
return () => {
// Revoke any blob URLs still held by uploading items
setLocalItems((prev) => {
prev.filter(i => i.status === "uploading" && i.previewUrl?.startsWith("blob:"))
.forEach(i => URL.revokeObjectURL(i.previewUrl!));
return prev;
});
};
}, []);Remove dead public upload fields, enforce mutually exclusive upload modes at the type level, and harden async object URL lifecycle cleanup to avoid unmount-related leaks while keeping behavior docs and tests in sync. Made-with: Cursor
commit: |
|
/review |
|
@copilot fix the conflict |
# Conflicts: # CLAUDE.md Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
docs/; adopted that simplified approach