Skip to content

【Auto】Fix Upload: toggle visible 后受控 fileList 的 blob URL 失效（ERR_FILE_NOT_FOUND） #3130

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

Open
SudoUserReal wants to merge 1 commit into
base: main
Choose a base branch
from
+260 −15
Open

【Auto】Fix Upload: toggle visible 后受控 fileList 的 blob URL 失效（ERR_FILE_NOT_FOUND） #3130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 136 additions & 14 deletions packages/semi-foundation/upload/foundation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export interface UploadAdapter<P = Record<string, any>, S = Record<string, any>>
notifyBeforeRemove: (file: BaseFileItem, fileList: Array<BaseFileItem>) => boolean | Promise<boolean>;
notifyBeforeClear: (fileList: Array<BaseFileItem>) => boolean | Promise<boolean>;
notifyChange: ({ currentFile, fileList }: { currentFile: BaseFileItem | null; fileList: Array<BaseFileItem> }) => void;
// Keep this as Array<string> for backward compatibility (external type stability).
// Internal logic uses uid->url map to avoid setState async timing issues.
updateLocalUrls: (urls: Array<string>) => void;
notifyClear: () => void;
notifyPreviewClick: (file: any) => void;
Expand All @@ -95,28 +97,102 @@ export interface UploadAdapter<P = Record<string, any>, S = Record<string, any>>

class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends BaseFoundation<UploadAdapter<P, S>, P, S> {
destroyState: boolean = false;
/**
* Canonical storage for objectURL created by this Upload instance.
* Do NOT rely on React state localUrls because setState is async and may lose updates
* when _createURL is called multiple times in a sync loop.
*/
_localUrls: Record<string, string> = {};
constructor(adapter: UploadAdapter<P, S>) {
super({ ...adapter });
}

init(): void {
// make sure state reset, otherwise may cause upload abort in React StrictMode, like https://github.com/DouyinFE/semi-design/pull/843
this.destroyState = false;
// In controlled mode, parent may keep and pass back fileList with blob url.
// Sync them into internal map so later remove/clear can revoke correctly.
this._syncLocalUrlsFromFileList();
const { disabled, addOnPasting } = this.getProps();
if (addOnPasting && !disabled) {
this.bindPastingHandler();
}
}

/**
* Sync internal objectURL map from current fileList.
* Only track blob: url with an in-memory File instance.
*/
_syncLocalUrlsFromFileList(): void {
const { fileList } = this.getStates() as any;
const next: Record<string, string> = {};
if (!Array.isArray(fileList)) {
this._localUrls = {};
this._adapter.updateLocalUrls([]);
return;
}
fileList.forEach((item: any) => {
const uid = item && item.uid;
const url = item && item.url;
const fileInstance = item && item.fileInstance;
const hasFileCtor = typeof File !== 'undefined';
const isFile = hasFileCtor && fileInstance instanceof File;
if (uid && typeof url === 'string' && url.startsWith('blob:') && isFile) {
next[uid] = url;
}
});
this._localUrls = next;
this._adapter.updateLocalUrls(Object.values(this._localUrls));
}

destroy() {
const { disabled, addOnPasting } = this.getProps();
this.releaseMemory();
// Do NOT revoke objectURL on unmount.
// In controlled mode, parent may keep and pass back fileList with blob url;
// revoking here will cause preview/blob ERR_FILE_NOT_FOUND after remount.
if (!disabled) {
this.unbindPastingHandler();
}
this.destroyState = true;
}

/**
* Release objectURL created for a specific file uid
*/
_releaseFileUrl(uid: string): void {
if (!uid) {
return;
}
const url = this._localUrls && this._localUrls[uid];
if (!url) {
return;
}
this._releaseBlob(url);
const next = { ...(this._localUrls || {}) };
delete next[uid];
this._localUrls = next;
this._adapter.updateLocalUrls(Object.values(this._localUrls));
}

/**
* Release all objectURL created by this Upload instance.
* Only call this when files are truly removed (e.g. clear).
*/
_releaseAllFileUrls(): void {
const localUrls = this._localUrls;
if (!localUrls || typeof localUrls !== 'object') {
return;
}
Object.keys(localUrls).forEach(uid => {
const url = localUrls[uid];
if (url) {
this._releaseBlob(url);
}
});
this._localUrls = {};
this._adapter.updateLocalUrls([]);
}

getError({ action, xhr, message, fileName }: { action: string;xhr: XMLHttpRequest;message?: string;fileName: string }): XhrError {
const status = xhr ? xhr.status : 0;
const msg = message || `cannot post ${fileName} to ${action}, xhr status: ${status}'`;
Expand Down Expand Up @@ -272,6 +348,12 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
this._adapter.notifyFileSelect([newFile]);
const newFileItem = this.buildFileItem(newFile, uploadTrigger);
const newFileList = [...fileList];

// replace an item, release its previous objectURL
const oldItem = newFileList[replaceIdx];
if (oldItem && oldItem.uid) {
this._releaseFileUrl(oldItem.uid);
}
newFileList.splice(replaceIdx, 1, newFileItem);
this._adapter.notifyChange({ currentFile: newFileItem, fileList: newFileList });
this._adapter.updateFileList(newFileList, () => {
Expand All @@ -297,7 +379,7 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
uid: fileInstance.uid,
percent: 0,
fileInstance,
url: this._createURL(fileInstance),
url: this._createURL(fileInstance, fileInstance.uid),
};

if (_sizeInvalid) {
Expand Down Expand Up @@ -497,6 +579,7 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
const { shouldUpload, status, autoRemove, validateMessage, fileInstance } = buResult;
let newFileList: Array<BaseFileItem> = this.getState('fileList').slice();
if (autoRemove) {
this._releaseFileUrl(file.uid);
newFileList = newFileList.filter(item => item.uid !== file.uid);
} else {
const index = this._getFileIndex(file, newFileList);
Expand All @@ -510,7 +593,9 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
newFileList[index].fileInstance = fileInstance;
newFileList[index].size = getFileSize(fileInstance.size);
newFileList[index].name = fileInstance.name;
newFileList[index].url = this._createURL(fileInstance);
// replace preview url, release old one first
this._releaseFileUrl(file.uid);
newFileList[index].url = this._createURL(fileInstance, file.uid);
}
newFileList[index].shouldUpload = shouldUpload;
}
Expand Down Expand Up @@ -674,8 +759,15 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
status ? (newFileList[index].status = status) : null;
validateMessage ? (newFileList[index].validateMessage = validateMessage) : null;
name ? (newFileList[index].name = name) : null;
url ? (newFileList[index].url = url) : null;
autoRemove ? newFileList.splice(index, 1) : null;
if (url) {
// if user replaces url, release local objectURL
this._releaseFileUrl(newFileList[index].uid);
newFileList[index].url = url;
}
if (autoRemove) {
this._releaseFileUrl(newFileList[index].uid);
newFileList.splice(index, 1);
}
}
this._adapter.notifySuccess(body, fileInstance, newFileList);
this._adapter.notifyChange({ fileList: newFileList, currentFile: newFileList[index] });
Expand Down Expand Up @@ -706,6 +798,8 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
}
newFileList.splice(index, 1);

this._releaseFileUrl(file.uid);

this._adapter.notifyRemove(file.fileInstance, newFileList, file);
this._adapter.updateFileList(newFileList);
this._adapter.notifyChange({ fileList: newFileList, currentFile: file });
Expand Down Expand Up @@ -742,6 +836,7 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
if (res === false) {
return;
}
this._releaseAllFileUrls();
this._adapter.updateFileList([]);
this._adapter.notifyClear();
this._adapter.notifyChange({ fileList: [] } as any);
Expand All @@ -750,23 +845,50 @@ class UploadFoundation<P = Record<string, any>, S = Record<string, any>> extends
});
}

_createURL(fileInstance: CustomFile): string {
_createURL(fileInstance: CustomFile, uid: string): string {
// https://stackoverflow.com/questions/31742072/filereader-vs-window-url-createobjecturl
const url = URL.createObjectURL(fileInstance);
const { localUrls } = this.getStates();
const newUrls = localUrls.slice();
newUrls.push(url);
this._adapter.updateLocalUrls(newUrls);
const next = { ...(this._localUrls || {}) };
// if there is an old url for this uid, release it first
if (uid && next[uid] && next[uid] !== url) {
this._releaseBlob(next[uid]);
}
if (uid) {
next[uid] = url;
}
this._localUrls = next;
// keep adapter/state in sync as a snapshot (legacy array type)
this._adapter.updateLocalUrls(Object.values(this._localUrls));
return url;
}

// 释放预览文件所占用的内存
// Release memory used by preview files
releaseMemory(): void {
const { localUrls }: { localUrls: Array<string> } = this.getStates();
localUrls.forEach(url => {
this._releaseBlob(url);
});
// Prefer internal map (canonical). Keep old state-based fallback for safety.
if (this._localUrls && Object.keys(this._localUrls).length) {
this._releaseAllFileUrls();
return;
}

const { localUrls } = this.getStates() as any;
if (!localUrls) {
return;
}
if (Array.isArray(localUrls)) {
localUrls.forEach(url => {
this._releaseBlob(url);
});
return;
}
if (typeof localUrls === 'object') {
Object.keys(localUrls).forEach(uid => {
const url = localUrls[uid];
if (url) {
this._releaseBlob(url);
}
});
}
}

_releaseBlob(url: string): void {
Expand Down
122 changes: 122 additions & 0 deletions packages/semi-ui/upload/__test__/upload.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sleep from '@douyinfe/semi-ui/_test_/utils/function/sleep';
import React from 'react';
import { IconUser } from '@douyinfe/semi-icons';
import { Upload, Button } from '../../index';
import { BASE_CLASS_PREFIX } from '../../../semi-foundation/base/constants';
Expand Down Expand Up @@ -68,12 +69,14 @@ describe('Upload', () => {
let requests;
let xhr;
window.URL.createObjectURL = jest.fn();
window.URL.revokeObjectURL = jest.fn();

beforeEach(() => {
xhr = sinon.useFakeXMLHttpRequest();
requests = [];
xhr.onCreate = req => requests.push(req);
window.URL.createObjectURL.mockReset();
window.URL.revokeObjectURL.mockReset();
});

afterEach(() => {
Expand Down Expand Up @@ -567,6 +570,125 @@ describe('Upload', () => {
upload.unmount();
});

it('should not revoke objectURL on unmount in controlled mode', done => {
// Simulate issue scenario: controlled fileList + Upload unmount/remount
const objUrl = 'blob:semi-upload-test-url';
window.URL.createObjectURL.mockImplementation(() => objUrl);

class ControlledWrapper extends React.Component {
constructor(props) {
super(props);
this.state = {
fileList: [],
visible: true,
};
}
toggle = () => {
this.setState(prev => ({ visible: !prev.visible }));
};
onChange = ({ fileList }) => {
this.setState({ fileList });
};
render() {
const { visible, fileList } = this.state;
return (
<div>
<button className="toggle" onClick={this.toggle}>toggle</button>
{visible ? (
<Upload action={action} fileList={fileList} onChange={this.onChange} showUploadList>
<Button>Upload</Button>
</Upload>
) : null}
</div>
);
}
}

const wrapper = mount(<ControlledWrapper />);
const upload = wrapper.find(Upload).at(0);
// select a file to create objectURL
const event = { target: { files: [createFile(20, 'toggle.png', 'image/png')] } };
trigger(upload, event);

expect(window.URL.createObjectURL).toHaveBeenCalled();
// toggle to unmount Upload
wrapper.find('button.toggle').simulate('click');
wrapper.update();
// should not revoke on unmount
setTimeout(() => {
expect(window.URL.revokeObjectURL).not.toHaveBeenCalled();
// toggle back to mount Upload again
wrapper.find('button.toggle').simulate('click');
wrapper.update();
expect(wrapper.find(Upload).exists()).toEqual(true);
// fileList still kept in wrapper state
expect(wrapper.state('fileList').length).toEqual(1);
wrapper.unmount();
done();
});
});

it('should revoke objectURL when remove/clear', done => {
const objUrl = 'blob:semi-upload-test-url-2';
window.URL.createObjectURL.mockImplementation(() => objUrl);

// remove
const upload = getUpload({});
trigger(upload, { target: { files: [createFile(10, 'remove.png', 'image/png')] } });
expect(window.URL.createObjectURL).toHaveBeenCalled();
upload.update();
upload.find(`button.${BASE_CLASS_PREFIX}-upload-file-card-close`).at(0).simulate('click', {});

setTimeout(() => {
expect(window.URL.revokeObjectURL).toHaveBeenCalled();

// clear
window.URL.revokeObjectURL.mockReset();
const upload2 = getUpload({ limit: 3 });
trigger(upload2, { target: { files: [createFile(10, 'clear.png', 'image/png')] } });
upload2.update();
const clearBtn = upload2.find(`.${BASE_CLASS_PREFIX}-upload-file-list-title-clear`).at(0);
// showTitle needs limit !== 1 and fileList.length
expect(clearBtn.exists()).toEqual(true);
clearBtn.simulate('click', {});

setTimeout(() => {
expect(window.URL.revokeObjectURL).toHaveBeenCalled();
upload.unmount();
upload2.unmount();
done();
});
});
});

it('should revoke all objectURLs when selecting multiple files then clear', done => {
const urlMap = {
'a.png': 'blob:multi-a',
'b.png': 'blob:multi-b',
};
window.URL.createObjectURL.mockImplementation(file => urlMap[file.name] || `blob:${file.name}`);

const upload = getUpload({ limit: 3 });
const fileA = createFile(10, 'a.png', 'image/png');
const fileB = createFile(10, 'b.png', 'image/png');
trigger(upload, { target: { files: [fileA, fileB] } });

expect(window.URL.createObjectURL).toHaveBeenCalledTimes(2);
upload.update();

const clearBtn = upload.find(`.${BASE_CLASS_PREFIX}-upload-file-list-title-clear`).at(0);
expect(clearBtn.exists()).toEqual(true);
clearBtn.simulate('click', {});

setTimeout(() => {
expect(window.URL.revokeObjectURL).toHaveBeenCalledTimes(2);
const revoked = window.URL.revokeObjectURL.mock.calls.map(args => args[0]);
expect(revoked).toEqual(expect.arrayContaining(['blob:multi-a', 'blob:multi-b']));
upload.unmount();
done();
});
});

it('afterUpload', () => {
// afterUploadResult:
// {
Expand Down
Loading
Loading