fix: resolve issue with grid reload parameter not working #4888
Conversation
|
WalkthroughThe pull request includes updates to several files within the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/effects/plugins/src/vxe-table/extends.tsOops! Something went wrong! :( ESLint: 9.14.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
packages/effects/plugins/src/vxe-table/extends.ts (1)
32-32: Consider adding type constraints for better type safetyWhile the current implementation works, we could improve type safety by constraining the
Recordabletype to specific expected properties.- const configFn = (proxyConfig?.ajax as Recordable<any>)?.[key]; + type ProxyParams = Recordable<string | number | boolean>; + const configFn = (proxyConfig?.ajax as Recordable<Function>)?.[key]; - params: Recordable<any>, - customValues: Recordable<any>, - ...args: Recordable<any>[] + params: ProxyParams, + customValues: ProxyParams, + ...args: unknown[]Also applies to: 38-40
pnpm-workspace.yaml (1)
26-26: Document the rationale for dependency updates.Several dependencies have been updated:
@iconify/json: ^2.2.271@tanstack/vue-store: ^0.5.7@vitejs/plugin-vue: ^5.2.0eslint-plugin-jsdoc: ^50.5.0happy-dom: ^15.11.4rollup: ^4.26.0vite-plugin-pwa: ^0.21.0vxe-pc-ui: ^4.2.53vxe-table: ^4.8.14Please document in the PR description:
- Which updates are essential for fixing the grid reload parameter issue
- Which updates are routine maintenance
- Any breaking changes that need attention
This documentation helps with future maintenance and troubleshooting.
Also applies to: 40-40, 56-56, 92-92, 108-108, 140-140, 167-167, 177-177, 178-178
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json(1 hunks)packages/effects/plugins/src/vxe-table/extends.ts(3 hunks)pnpm-workspace.yaml(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (2)
packages/effects/plugins/src/vxe-table/extends.ts (2)
37-50: Fix confirmed: Parameters now correctly passed to query method
The implementation now properly merges customValues with formValues using the spread operator, which resolves the issue reported in #4886 where parameters passed to reload weren't reaching the query method.
1-1: Verify the Recordable type definition and its implications
The change from Record<string, any> to Recordable<any> seems consistent, but we should verify the type definition to ensure it maintains the expected behavior.
Also applies to: 11-11, 29-29
✅ Verification successful
</run_scripts>
Recordable type definition is correct and consistent
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Recordable type definition
rg -t typescript "type\s+Recordable" -A 3
Length of output: 78
Script:
#!/bin/bash
# Search for Recordable type definition in TypeScript files
rg "type\s+Recordable" --glob "*.ts" --glob "*.tsx" -A 3
Length of output: 294
Description
fixed #4886
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
Chores