-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
fix(Teleport): hydrate disabled Teleport with undefined target #11235
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
base: main
Are you sure you want to change the base?
Conversation
…eleport should use the node of nextSibling
Hello, thanks for the PR. Is there any chance it will get merged? |
@@ -406,6 +406,21 @@ function hydrateTeleport( | |||
8000 | } | ||
updateCssVars(vnode) | |||
} | |||
if (!target && isTeleportDisabled(vnode.props)) { |
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.
if (!target && isTeleportDisabled(vnode.props)) { | |
else if (isTeleportDisabled(vnode.props)) { | |
if (vnode.shapeFlag & ShapeFlags.ARRAY_CHILDREN) { | |
vnode.anchor = hydrateChildren( | |
nextSibling(node), | |
vnode, | |
parentNode(node)!, | |
parentComponent, | |
parentSuspense, | |
slotScopeIds, | |
optimized, | |
) | |
vnode.targetStart = node | |
vnode.targetAnchor = nextSibling(node) | |
} | |
} |
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.
Please try to extract a hydrateDisabledTeleport
method.
Could you please add a test case?
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.
Thank you for your review, I will improve it
Size ReportBundles
Usages
|
bd3ac0e
to
49581b7
Compare
Sorry for the bump, but is there any chance it will get merged? Thanks a lot for your work @linzhe141 🙏 |
Maybe the current modification is not correct, so it may not be merged. |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
@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: |
Any update on this one? There's still dependencies relying on this changes 🙏 |
WalkthroughA new test was added to verify SSR hydration of a disabled Teleport with an undefined target. The Teleport hydration logic was updated to explicitly handle disabled state both with and without a resolved target, ensuring correct vnode property assignments and hydration of children in these scenarios. Changes
Sequence Diagram(s)sequenceDiagram
participant SSR as Server
participant DOM as DOM Container
participant App as Vue App
participant Teleport as Teleport Component
SSR->>DOM: Render Teleport (disabled, target undefined) to string
App->>DOM: Mount app on container
App->>Teleport: Initialize Teleport (disabled, target undefined)
Teleport->>Teleport: Detect disabled & undefined target
Teleport->>Teleport: Hydrate children in place (using parent container)
Teleport->>App: Complete hydration
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🔇 Additional comments (5)
✨ 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 (
|
close #11230
Summary by CodeRabbit
New Features
Tests