fix(file-upload): fix file upload size limit and the vue2 date-panel example #3777
Conversation
WalkthroughEight Vue demo components refactored from Composition API Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The changes follow consistent, repetitive refactoring patterns across multiple files (Vue Options API conversion) with straightforward type safety enhancements. Low logic density and homogeneous transformation patterns reduce cognitive load. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/renderless/src/file-upload/index.ts (1)
402-402: RedundantMath.minwith single argument.Calling
Math.min(state.singleMaxSize)with a single argument simply returns that argument. This is functionally correct but unnecessarily verbose.Simplify by removing the redundant function call:
- maxSize = state.isEdm ? Math.min(state.singleMaxSize) : Number(props.fileSize) / 1024 + maxSize = state.isEdm ? state.singleMaxSize : Number(props.fileSize) / 1024examples/sites/demos/pc/app/date-panel/custom-weeks.vue (1)
33-33: Remove unusedeachWeekFirstDayproperty.The
eachWeekFirstDayproperty is assigned in theformatWeeksmethod (line 38) but never referenced in the template. Unlike the similar component indate-picker/custom-weeks.vuewhich displays this value, thedate-panelvariant does not use it. Remove the property declaration and assignment to keep the component clean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/sites/demos/pc/app/date-panel/basic-usage.vue(1 hunks)examples/sites/demos/pc/app/date-panel/custom-weeks.vue(1 hunks)examples/sites/demos/pc/app/date-panel/disabled-date.vue(1 hunks)examples/sites/demos/pc/app/date-panel/event.vue(1 hunks)examples/sites/demos/pc/app/date-panel/format.vue(1 hunks)examples/sites/demos/pc/app/date-panel/readonly.vue(1 hunks)examples/sites/demos/pc/app/date-panel/shortcuts.vue(1 hunks)examples/sites/demos/pc/app/date-panel/unlink-panels.vue(1 hunks)packages/renderless/src/file-upload/index.ts(1 hunks)
🔇 Additional comments (9)
packages/renderless/src/file-upload/index.ts (1)
399-400: Good type safety improvements withNumber()conversions.The explicit
Number()conversions ensure that arithmetic operations work correctly regardless of the input type. This properly addresses the file size limit enforcement issue mentioned in the PR.examples/sites/demos/pc/app/date-panel/custom-weeks.vue (1)
36-41: LGTM: Method conversion is correct.The
formatWeeksfunction has been properly converted from a standalone function to a component method, with correct use ofthisfor accessing component properties.examples/sites/demos/pc/app/date-panel/unlink-panels.vue (1)
21-37: LGTM: Clean Options API conversion.The refactoring correctly moves reactive state to
data()and explicitly registers components. All data properties are properly used in the template.examples/sites/demos/pc/app/date-panel/basic-usage.vue (1)
18-38: LGTM: Options API conversion is structurally correct.The refactoring properly moves all reactive state to
data()and registers components explicitly.However, note that line 6 of the template uses
v-model="value"for a month-type panel, while themonthdata property exists but isn't bound to that panel. This appears to be a pre-existing issue not addressed in this PR.examples/sites/demos/pc/app/date-panel/readonly.vue (1)
18-38: LGTM: Clean Options API conversion.The refactoring correctly moves reactive state to
data()and explicitly registers all components. All data properties are properly used in the template.examples/sites/demos/pc/app/date-panel/disabled-date.vue (1)
38-48: LGTM: Methods correctly converted.The disabled date methods have been properly moved from standalone functions to component methods. The logic is preserved from the original implementation.
examples/sites/demos/pc/app/date-panel/event.vue (1)
38-57: LGTM: Event handlers correctly converted.All event handler methods have been properly moved to the component's methods section. The modal message logic is preserved correctly.
examples/sites/demos/pc/app/date-panel/format.vue (1)
12-28: LGTM: Clean Options API conversion.The refactoring correctly moves reactive state to
data()and explicitly registers components. The date format in the data matches the format specified in the template.examples/sites/demos/pc/app/date-panel/shortcuts.vue (1)
21-203: LGTM: Large shortcuts conversion handled correctly.The extensive shortcuts arrays have been properly moved from standalone constants into the component's
data()return object. The overall structure and date manipulation logic is preserved.
| text: '本年', | ||
| onClick(picker) { | ||
| const date = new Date() | ||
| const tmp = new Date(date.getFullYear(), 0, 2) | ||
| const start = new Date(date.getFullYear(), 0, 2) | ||
| const end = new Date(tmp.getTime() - 1) | ||
| picker.$emit('pick', [start, end]) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify date range logic for "本年" (this year) shortcut.
The logic appears to create a backwards date range:
startis set to January 2 of the current year (line 177)endis calculated as 1ms before January 2, which is effectively January 1 (line 179)
This results in start > end, which is likely incorrect for a year range shortcut. Typically, "本年" should span from January 1 to December 31 of the current year, or from January 1 to today.
Consider this fix:
{
text: '本年',
onClick(picker) {
const date = new Date()
- const tmp = new Date(date.getFullYear(), 0, 2)
- const start = new Date(date.getFullYear(), 0, 2)
- const end = new Date(tmp.getTime() - 1)
+ const start = new Date(date.getFullYear(), 0, 1)
+ const end = new Date(date.getFullYear(), 11, 31)
picker.$emit('pick', [start, end])
}
}I'll verify the date range logic issue by examining the actual file and checking similar patterns in the codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# 1. Display the relevant section of the file
echo "=== Checking lines 174-181 in shortcuts.vue ==="
sed -n '170,185p' examples/sites/demos/pc/app/date-panel/shortcuts.vue
# 2. Look for other year/date shortcuts to understand the pattern
echo -e "\n=== Looking for similar date patterns in the file ==="
grep -n "getFullYear\|0, 1\|11, 31\|12, 31" examples/sites/demos/pc/app/date-panel/shortcuts.vue | head -20
# 3. Check if there's a "上年" or similar pattern we can compare against
echo -e "\n=== Searching for all shortcuts with similar structure ==="
sed -n '1,200p' examples/sites/demos/pc/app/date-panel/shortcuts.vue | grep -A 5 "text:"
Length of output: 5237
Fix backwards date range in "本年" (this year) shortcut.
The issue is confirmed. The code creates an invalid date range where start (January 2) is after end (January 1):
- Line 177:
tmpis set to January 2 - Line 178:
startis set to January 2 (same astmp) - Line 179:
endis 1ms before January 2, which is effectively January 1
This backwards range is inconsistent with the "本月" (this month) pattern in the same file (lines 147-148), which correctly creates a valid range by calculating the next period and subtracting 1ms.
For "本年", this should span the full calendar year (January 1 to December 31):
{
text: '本年',
onClick(picker) {
const date = new Date()
- const tmp = new Date(date.getFullYear(), 0, 2)
- const start = new Date(date.getFullYear(), 0, 2)
- const end = new Date(tmp.getTime() - 1)
+ const start = new Date(date.getFullYear(), 0, 1)
+ const end = new Date(date.getFullYear(), 11, 31)
picker.$emit('pick', [start, end])
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| text: '本年', | |
| onClick(picker) { | |
| const date = new Date() | |
| const tmp = new Date(date.getFullYear(), 0, 2) | |
| const start = new Date(date.getFullYear(), 0, 2) | |
| const end = new Date(tmp.getTime() - 1) | |
| picker.$emit('pick', [start, end]) | |
| } | |
| text: '本年', | |
| onClick(picker) { | |
| const date = new Date() | |
| const start = new Date(date.getFullYear(), 0, 1) | |
| const end = new Date(date.getFullYear(), 11, 31) | |
| picker.$emit('pick', [start, end]) | |
| } |
🤖 Prompt for AI Agents
In examples/sites/demos/pc/app/date-panel/shortcuts.vue around lines 174 to 181,
the "本年" shortcut builds a backwards range by setting both tmp and start to
January 2 and end to 1ms before that; change it so start is the first
millisecond of Jan 1 of the current year and end is the last millisecond of Dec
31 of the current year (e.g. set start = new Date(year, 0, 1) and end = new
Date(year + 1, 0, 1).getTime() - 1) so the picker receives [start, end] as a
proper full-calendar-year range.
| let maxSize = 0 | ||
|
|
||
| state.singleMaxSize = props.edm.singleFileMaxSize || state.singleMaxSize || 200 | ||
| if (Array.isArray(props.fileSize) && props.fileSize[1]) { |
There was a problem hiding this comment.
Edge case: fileSize[1] check fails when value is 0.
The condition props.fileSize[1] evaluates to false when the value is 0, which might be a valid max file size. This could cause the logic to incorrectly fall through to the else branch.
Consider using an explicit check for undefined or null:
- if (Array.isArray(props.fileSize) && props.fileSize[1]) {
+ if (Array.isArray(props.fileSize) && props.fileSize[1] !== undefined) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (Array.isArray(props.fileSize) && props.fileSize[1]) { | |
| if (Array.isArray(props.fileSize) && props.fileSize[1] !== undefined) { |
🤖 Prompt for AI Agents
In packages/renderless/src/file-upload/index.ts around line 397, the condition
`Array.isArray(props.fileSize) && props.fileSize[1]` incorrectly treats a valid
max size of 0 as falsy; change the check to explicitly test for undefined/null
(e.g., `Array.isArray(props.fileSize) && props.fileSize[1] != null` or `!==
undefined`) so that 0 is accepted as a valid value and the correct branch
executes.
2 participants
PR
PR Checklist
fix:修复文件上传大小限制失效，以及vue2时间面板示例
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Refactor
Bug Fixes