fix(hydration): handle transition appear hydration edge case#13339
fix(hydration): handle transition appear hydration edge case#13339edison1105 merged 3 commits intomainfrom
Conversation
|
""" WalkthroughThe changes implement a caching mechanism for the Changes
Sequence Diagram(s)sequenceDiagram
participant Hydrator as Hydration Process
participant Template as <template> Element
participant Child as First Child Element
participant MismatchCheck as propHasMismatch
Hydrator->>Template: Hydrate content
Template->>Child: Assign $cls with initial class attribute
Hydrator->>MismatchCheck: Check for class mismatch
MismatchCheck->>Child: Retrieve $cls (if present)
MismatchCheck->>Child: Fallback to class attribute if $cls not present
Assessment against linked issues
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Size ReportBundles
Usages
|
@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: |
f5998fb to
6754842
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/hydration.ts (1)
803-808: Consider using assignment instead of delete for better performance.The implementation correctly prioritizes the cached class value over the current attribute, but the
deleteoperator can impact performance.if (el.$cls) { actual = el.$cls - delete el.$cls + el.$cls = undefined } else { actual = el.getAttribute('class') }🧰 Tools
🪛 Biome (1.9.4)
[error] 805-805: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
packages/runtime-core/__tests__/hydration.spec.ts(1 hunks)packages/runtime-core/src/hydration.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-core/tests/hydration.spec.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/runtime-core/src/hydration.ts
[error] 805-805: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test / unit-test-windows
🔇 Additional comments (3)
packages/runtime-core/src/hydration.ts (3)
401-402: Type extension appropriately handles class caching.Good job extending the element type to include the
$clsproperty, which will be used to store the original class value during hydration, addressing the transition appear edge case.
404-405: LGTM: Caching class value before transition hooks.Excellent solution to store the original class value in
$clsbefore the transition'sbeforeEnterhook is called. This ensures we preserve the original class for accurate hydration comparison.
789-791: Type signature properly updated.The type signature for
propHasMismatchhas been correctly updated to include the optional$clsproperty, maintaining type safety throughout the codebase.
501ba43 to
1a1a63d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/hydration.ts (1)
804-809: Smart handling of class attribute for hydration comparison.This logic addresses the edge case by:
- First checking for the cached class value from
$cls- Using it for hydration comparison if available
- Cleaning up by removing the property after use
- Falling back to the attribute method when no cache exists
There's a minor performance consideration with the
deleteoperator, but since this is:
- Only used during hydration (not in hot path)
- A one-time operation per element
- Necessary to clean up the temporary property
The current implementation is appropriate for this use case.
For maximum performance, you could consider using
el.$cls = undefinedinstead ofdelete el.$cls, but the difference is negligible in this context.🧰 Tools
🪛 Biome (1.9.4)
[error] 806-806: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/runtime-core/src/hydration.ts(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/runtime-core/src/hydration.ts
[error] 806-806: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (3)
packages/runtime-core/src/hydration.ts (3)
401-401: Type extension enhances template element's first child.This type extension adds support for the
$clsproperty which will be used to cache the class attribute value during hydration. This change is crucial for handling transition appear cases correctly.
404-406: Good implementation for caching class value before transition hooks.This code properly caches the original class attribute value before the transition's
beforeEnterhook can modify it. This elegantly solves the hydration mismatch issue that can occur with transition appear.
791-791: Type signature updated for consistency with implementation.The parameter type is properly updated to allow for the
$clsproperty on elements, ensuring type safety across the codebase.
close #13335
Summary by CodeRabbit