-
Notifications
You must be signed in to change notification settings - Fork 7.1k
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.
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
chore: vxe-table's renderDefault is deprecated, use renderTableDefault #4814
Conversation
WalkthroughThe changes across multiple Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
apps/web-antd/src/adapter/vxe-table.ts (1)
Line range hint
38-56
: Consider enhancing renderer documentation.While the code changes are correct, the renderer documentation could be improved by:
- Documenting the expected parameters and their types
- Adding example usage with different props
Example documentation improvement:
// 表格配置项可以用 cellRender: { name: 'CellImage' }, + /** + * Renders an image in a table cell + * @param renderOpts - Renderer options + * @param params.column - Column configuration containing field name + * @param params.row - Row data containing the image URL + * @example + * { + * cellRender: { + * name: 'CellImage', + * props: { /* additional Image component props */ } + * } + * } + */playground/src/adapter/vxe-table.ts (1)
Line range hint
50-57
: LGTM! Consider making button size configurable.The update to
renderTableDefault
is correct. However, we could improve flexibility by making the button size configurable through props.Consider this enhancement:
renderTableDefault(renderOpts) { const { props } = renderOpts; return h( Button, - { size: 'small', type: 'link' }, + { size: props?.size || 'small', type: 'link' }, { default: () => props?.text }, ); },apps/web-naive/src/adapter/vxe-table.ts (1)
Line range hint
49-56
: LGTM! Correctly updated to use renderTableDefault.The change from
renderDefault
torenderTableDefault
addresses the deprecation while maintaining the same functionality.Consider adding type safety for the props. You could define an interface for the expected props:
interface CellLinkProps { text?: string; }Then update the renderOpts parameter:
-renderTableDefault(renderOpts) { +renderTableDefault(renderOpts: { props: CellLinkProps }) {apps/web-ele/src/adapter/vxe-table.ts (1)
Line range hint
50-56
: LGTM! Consider adding type safety improvements.The change from
renderDefault
torenderTableDefault
is correct. However, we could improve type safety for the props handling.Consider adding proper typing for the renderOpts parameter:
- renderTableDefault(renderOpts) { + renderTableDefault(renderOpts: { props: { text?: string } }) {This would provide better TypeScript support and catch potential issues at compile time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/web-antd/src/adapter/vxe-table.ts
(1 hunks)apps/web-ele/src/adapter/vxe-table.ts
(2 hunks)apps/web-naive/src/adapter/vxe-table.ts
(1 hunks)playground/src/adapter/vxe-table.ts
(1 hunks)
🔇 Additional comments (5)
apps/web-antd/src/adapter/vxe-table.ts (2)
Line range hint 49-56
: LGTM! Method name updated correctly.
The change from renderDefault
to renderTableDefault
is properly implemented while maintaining the existing button rendering functionality.
41-44
: LGTM! Method name updated correctly.
The change from renderDefault
to renderTableDefault
aligns with vxe-table's deprecation notice while preserving the existing rendering logic.
Let's verify if there are any other instances of renderDefault
that need to be updated:
✅ Verification successful
✅ All instances already use the updated renderTableDefault
method
The search results show that all vxe-table renderer implementations across different apps (web-antd, web-naive, web-ele, and playground) are already using the new renderTableDefault
method. No instances of the deprecated renderDefault
were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of renderDefault in the codebase
rg "renderDefault" --type ts
Length of output: 28
Script:
#!/bin/bash
# Let's try a more comprehensive search with ripgrep
# Search for both old and new method names to understand the context better
rg -i "render(Default|TableDefault)" --type ts -A 2 -B 2
Length of output: 3244
playground/src/adapter/vxe-table.ts (1)
Line range hint 42-57
: Verify consistent updates across all apps.
Since this is a breaking change from vxe-table, we should ensure all apps in the monorepo have been updated consistently.
Let's verify the changes across all apps:
✅ Verification successful
All apps have been updated consistently with the new renderTableDefault method ✅
The verification shows that all apps in the monorepo (web-antd
, web-ele
, web-naive
, and playground
) have been properly updated to use renderTableDefault
. There are no remaining instances of the old renderDefault
method, confirming that the breaking change has been handled consistently across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining uses of renderDefault in vxe-table files
rg "renderDefault" --type ts "apps/**/vxe-table.ts"
# Verify all apps have the new renderTableDefault
rg "renderTableDefault" --type ts "apps/**/vxe-table.ts"
Length of output: 230
Script:
#!/bin/bash
# Let's first find all vxe-table related files in the repository
fd "vxe-table" --type file
# Then search for renderDefault and renderTableDefault in all TypeScript files
rg "renderDefault" --type ts
rg "renderTableDefault" --type ts
Length of output: 990
apps/web-naive/src/adapter/vxe-table.ts (1)
41-44
: LGTM! Correctly updated to use renderTableDefault.
The change from renderDefault
to renderTableDefault
addresses the deprecation while maintaining the same functionality.
Let's verify if there are any other instances of renderDefault
that need to be updated:
✅ Verification successful
All instances of renderDefault
have been properly updated to renderTableDefault
The search results confirm that:
- All instances of the render method across different UI adapters (naive, antd, ele) are using the new
renderTableDefault
name - No legacy
renderDefault
methods remain in the codebase - The changes are consistently applied in both the CellImage and CellLink renderers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of renderDefault in the codebase
rg "renderDefault" --type ts
Length of output: 28
Script:
#!/bin/bash
# Let's try a more comprehensive search with ast-grep to find any render method patterns
ast-grep --pattern 'render$_Default'
# Also search with ripgrep using word boundaries and case insensitive
rg -w -i 'render\w*default' --type ts
# And check the specific file content to understand the full context
cat "apps/web-naive/src/adapter/vxe-table.ts"
Length of output: 2376
apps/web-ele/src/adapter/vxe-table.ts (1)
Line range hint 41-45
: LGTM! Correctly updated to use renderTableDefault.
The change from renderDefault
to renderTableDefault
addresses the deprecation while maintaining the existing image rendering functionality.
Let's verify if there are any remaining usages of the deprecated renderDefault
:
✅ Verification successful
All instances of renderDefault
have been properly updated to renderTableDefault
The search results confirm that all vxe-table adapters across different UI frameworks (Element Plus, Naive UI, Ant Design) are consistently using the new renderTableDefault
method. There are no remaining instances of the deprecated renderDefault
method in the codebase. The changes are properly synchronized across all implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of renderDefault in the codebase
rg "renderDefault" --type ts
Length of output: 28
Script:
#!/bin/bash
# Let's try a more comprehensive search with different variations and context
rg -i "render.*default" --type ts -A 2
# Also search for any vxe-table related renderer configurations
ast-grep --pattern 'render$_Default($$$)'
Length of output: 2261
Summary by CodeRabbit