fix(antd Upload onChange Event): rewrite onChange event to handle upl… #7098
Conversation
…oad success or error messages
jyqwq
requested review from
a team,
anncwb,
jinmao88,
mynetfan and
vince292007
as code owners
|
📝 WalkthroughWalkthroughForward Upload change events to optional external handlers ( Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…oad success or error messages
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-antd/src/adapter/component/index.ts (1)
290-343: Filter outhandleChangeandonHandleChangefrom attrs before spreading to Upload, and guard external handler calls.With
inheritAttrsdefaulting totrue, spreading...attrsforwards these custom properties to theant-design-vueUpload component, which doesn't recognize them. This surfaces extraneous attribute warnings. Additionally, if an external handler throws, it will currently break yourfileListsync since the state update happens after the handler calls.Proposed fix
- const handleChange = async (event: UploadChangeParam) => { - // 行内写法 handleChange: (event) => {} - attrs.handleChange?.(event); - // template写法 @handle-change="(event) => {}" - attrs.onHandleChange?.(event); + const handleChange = (event: UploadChangeParam) => { + // 行内写法 handleChange: (event) => {} + // template写法 @handle-change="(event) => {}" + try { + attrs.handleChange?.(event); + attrs.onHandleChange?.(event); + } catch (e) { + // Avoid breaking internal v-model sync on user handler errors + console.error(e); + } fileList.value = event.fileList.filter( (file) => file.status !== 'removed', ); emit( 'update:modelValue', event.fileList?.length ? fileList.value : undefined, ); }; @@ - return () => + return () => { + const { handleChange: _hc, onHandleChange: _ohc, ...uploadAttrs } = + attrs as any; + return ( h( Upload, { ...props, - ...attrs, + ...uploadAttrs, fileList: fileList.value, beforeUpload: handleBeforeUpload, onChange: handleChange, onPreview: handlePreview, }, renderUploadButton(), - ); + ) + ); + };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web-antd/src/adapter/component/index.tsplayground/src/adapter/component/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
🔇 Additional comments (1)
playground/src/adapter/component/index.ts (1)
299-352: FilterhandleChangeandonHandleChangeout of forwardedattrsto prevent duplicate listener warnings.Currently, these handlers are both called directly and passed to the
Uploadcomponent via...attrs, which can trigger "extraneous attrs" warnings depending on howUploadis defined. Destructure attrs to exclude these properties before spreading, and wrap handler invocations in try-catch for safety:Proposed fix
- const handleChange = async (event: UploadChangeParam) => { - attrs.handleChange?.(event); - attrs.onHandleChange?.(event); + const handleChange = (event: UploadChangeParam) => { + try { + attrs.handleChange?.(event); + attrs.onHandleChange?.(event); + } catch (e) { + console.error(e); + } fileList.value = event.fileList.filter( (file) => file.status !== 'removed', ); emit( 'update:modelValue', event.fileList?.length ? fileList.value : undefined, ); }; + - return () => + return () => { + const { handleChange: _hc, onHandleChange: _ohc, ...uploadAttrs } = attrs; + return ( h( Upload, { ...props, - ...attrs, + ...uploadAttrs, fileList: fileList.value, beforeUpload: handleBeforeUpload, onChange: handleChange, onPreview: handlePreview, }, renderUploadButton(), - ); + ) + ); + };
…oad success or error messages
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
playground/src/adapter/component/index.ts (1)
299-316: LGTM — mirrors the pattern inapps/web-antdadapter.The implementation correctly isolates user handler errors. Since this logic is duplicated verbatim in
apps/web-antd/src/adapter/component/index.ts, you could consider extracting thehandleChangehelper to a shared utility to reduce duplication across adapters. This is optional and can be deferred if maintaining separate adapters is intentional.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web-antd/src/adapter/component/index.tsplayground/src/adapter/component/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
🔇 Additional comments (1)
apps/web-antd/src/adapter/component/index.ts (1)
290-307: LGTM — correctly isolates user handler errors from internal state sync.The try-catch ensures user-provided handlers don't break v-model synchronization. One minor note: if a user handler is
asyncand throws, the rejection occurs outside this synchronous try-catch and won't be caught. This is acceptable since the subsequent state update still executes unaffected, but consider documenting this behavior for consumers.
2 participants
…oad success or error messages
因为antd Upload已被重新封装，导致onChange事件存在冲突，会出现警告信息，通过切换为
handleChange事件解决此警告。Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.