fix(Transition): handle KeepAlive + transition leaving edge case #13152
Conversation
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
f11c251 to
16deaaf
Compare
4673e95 to
5481015
Compare
|
为啥还不合并发布呢 |
WalkthroughThe changes export the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant KeepAlive
participant Transition
participant Renderer
User->>App: Toggle v-show (start leave transition)
App->>Transition: Initiate leave
Transition->>Renderer: Perform leave animation
User->>App: Switch component before leave finishes
App->>KeepAlive: Move node
KeepAlive->>Renderer: move()
Renderer->>Renderer: Detect _isLeaving, call el[leaveCbKey](true)
Renderer->>DOM: Set display: none
Note right of Renderer: Ensures correct display state after switch
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/vue/__tests__/e2e/Transition.spec.ts (1)
1783-1822: Consider breaking down HTML assertions for better maintainability.The current approach uses string concatenation for HTML assertions, which can be brittle and hard to debug when tests fail. Consider breaking these into separate, more focused assertions:
- expect(await html('#container')).toBe( - `<div><h2>I should show</h2></div>` + - `<h2>This is page1</h2>` + - `<button id="changeShowBtn">true</button>`, - ) + // Verify each component separately for clearer failure messages + expect(await html('#container div:first-child')).toBe(`<div><h2>I should show</h2></div>`) + expect(await html('#container h2:nth-of-type(2)')).toBe(`<h2>This is page1</h2>`) + expect(await html('#container button')).toBe(`<button id="changeShowBtn">true</button>`)This approach provides:
- Clearer failure messages when specific elements don't match
- Better maintainability when DOM structure changes
- Easier debugging of test failures
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-core/src/components/BaseTransition.ts(1 hunks)packages/runtime-core/src/renderer.ts(2 hunks)packages/vue/__tests__/e2e/Transition.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/vue/__tests__/e2e/Transition.spec.ts (1)
packages/vue/__tests__/e2e/e2eUtils.ts (1)
E2E_TIMEOUT(8-8)
🔇 Additional comments (5)
packages/runtime-core/src/components/BaseTransition.ts (1)
27-27: LGTM! Clean export for external module access.The change from a private to public export of
leaveCbKeyenables the renderer to access this symbol for handling the KeepAlive + transition edge case. The symbol maintains type safety and follows Vue's architectural patterns.packages/runtime-core/src/renderer.ts (2)
88-88: LGTM! Import updated for the KeepAlive + transition fix.The import statement correctly includes the newly exported
leaveCbKeysymbol needed for the transition leave callback handling.
2073-2078: Excellent fix for the KeepAlive + transition edge case!This logic addresses the core issue described in #13153. When a kept-alive component is moved before a v-show transition leave finishes, it ensures the leaving callback is triggered early with cancellation (
true) to set the element'sdisplaystyle tonone. This prevents components from remaining visible when they should be hidden.The implementation is clean and follows Vue's transition patterns by:
- Checking the
_isLeavingproperty to identify elements in leaving state- Calling the leave callback with cancellation to ensure proper cleanup
- Integrating seamlessly with the existing transition system
packages/vue/__tests__/e2e/Transition.spec.ts (2)
1726-1825: Well-structured test case for the KeepAlive + v-show transition edge case.This test effectively covers the specific issue #13153 by:
- Setting up the exact scenario with KeepAlive, component switching, and v-show transitions
- Verifying the critical behavior where
display: noneis applied when switching components during a transition- Testing both directions of component switching to ensure consistency
The test structure is solid and the assertions are comprehensive, checking both DOM structure and inline styles at each step.
1790-1817: Proper timing and synchronization approach.The test correctly uses
nextTick()after state changes andtransitionFinish()to wait for CSS transitions, following the established patterns in the test suite. This ensures reliable test execution across different environments.
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
close #13153
Summary by CodeRabbit
Bug Fixes
Tests