-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
fix: vxeTable destruction/reconstruction remains at the first value #4805
Conversation
|
WalkthroughThe changes in this pull request modify the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/effects/plugins/src/vxe-table/use-vxe-grid.ts
(1 hunks)
🔇 Additional comments (2)
packages/effects/plugins/src/vxe-table/use-vxe-grid.ts (2)
17-21
: LGTM! This change properly fixes the state retention issue.
Creating a new api
instance and defining useStore
within the component scope ensures that:
- Each grid instance has its own isolated state
- Previous values aren't retained when the component is destroyed and reconstructed
- The store is properly bound to the correct api instance
This effectively resolves the issue described in #4804 where the grid was retaining initial values after destruction/reconstruction.
23-25
: Verify cleanup implementation.
Let's ensure the cleanup is properly implemented to prevent memory leaks.
✅ Verification successful
Let me analyze the cleanup implementation further by checking the StateHandler class since it's involved in the cleanup process.
Cleanup implementation is properly handled
The cleanup implementation in unmount()
is sufficient as it:
- Resets the mounted state via
isMounted = false
- Properly cleans up the state handler which includes clearing any pending promises and their callbacks
- No event listeners or subscriptions are created that would need cleanup
- The Store updates are handled through the component lifecycle
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify VxeGridApi cleanup implementation
# Expected: Find unmount method implementation and any cleanup logic
# Search for VxeGridApi class implementation
echo "Searching for VxeGridApi class implementation..."
ast-grep --pattern 'class VxeGridApi {
$$$
unmount() {
$$$
}
$$$
}'
# Search for additional cleanup related code
echo "Searching for additional cleanup patterns..."
rg -l "(?i)(cleanup|dispose|destroy|unmount)" --type ts
Length of output: 7810
Script:
#!/bin/bash
# Search for StateHandler class implementation to understand reset() method
ast-grep --pattern 'class StateHandler {
$$$
reset() {
$$$
}
$$$
}'
# Check if there are any event listeners or subscriptions that need cleanup
rg "addEventListener|on\(" packages/effects/plugins/src/vxe-table/
Length of output: 4455
Description
解决VxeTable在modal/drawer中使用(或使用v-if切换)一直拿到的是第一次的表单
resolve: #4804 有详细复现步骤及分析
#4752
#4785
Type of change
Please delete options that are not relevant.
pnpm-lock.yaml
unless you introduce a new test example.Checklist
pnpm run docs:dev
command.pnpm test
.feat:
,fix:
,perf:
,docs:
, orchore:
.Summary by CodeRabbit
extendedApi
variable accurately reflects the latest instance of the API, enhancing stability.