Skip to content
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(custom-element): handle nested customElement mount w/ shadowRoot false #11861

Merged
merged 18 commits into from
Sep 13, 2024

Conversation

linzhe141
Copy link
Contributor

@linzhe141 linzhe141 commented Sep 9, 2024

Copy link

github-actions bot commented Sep 9, 2024

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 100 kB (+108 B) 37.8 kB (+35 B) 34 kB (+95 B)
vue.global.prod.js 159 kB (+108 B) 57.7 kB (+41 B) 51.2 kB (-79 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 48.8 kB 18.8 kB 17.2 kB
createApp 55.3 kB 21.2 kB 19.4 kB
createSSRApp 59.3 kB 22.9 kB 20.9 kB
defineCustomElement 60.1 kB (+70 B) 22.8 kB (+25 B) 20.7 kB (+24 B)
overall 69 kB 26.3 kB 23.9 kB

Copy link

pkg-pr-new bot commented Sep 9, 2024

Open in Stackblitz

@vue/compiler-core

pnpm add https://pkg.pr.new/@vue/compiler-core@11861

@vue/compiler-dom

pnpm add https://pkg.pr.new/@vue/compiler-dom@11861

@vue/compiler-sfc

pnpm add https://pkg.pr.new/@vue/compiler-sfc@11861

@vue/compiler-ssr

pnpm add https://pkg.pr.new/@vue/compiler-ssr@11861

@vue/runtime-core

pnpm add https://pkg.pr.new/@vue/runtime-core@11861

@vue/reactivity

pnpm add https://pkg.pr.new/@vue/reactivity@11861

@vue/runtime-dom

pnpm add https://pkg.pr.new/@vue/runtime-dom@11861

@vue/shared

pnpm add https://pkg.pr.new/@vue/shared@11861

@vue/server-renderer

pnpm add https://pkg.pr.new/@vue/server-renderer@11861

@vue/compat

pnpm add https://pkg.pr.new/@vue/compat@11861

vue

pnpm add https://pkg.pr.new/vue@11861

commit: 7ff7496

@linzhe141 linzhe141 changed the title fix(runtime-dom): the _parseSlots of customElement(shadowRoot:false) executed correctly fix(runtime-dom): the _parseSlots of customElement(shadowRoot:false) should executed correctly Sep 9, 2024
@edison1105
Copy link
Member

edison1105 commented Sep 9, 2024

It seems the problem related to Teleport has not been resolved. see Playground with this PR
To fix this problem, it needs to use the teleport target to query all slots.

Suggestion

  • Pass the teleport target to the ce during the teleport mounting
    if (shapeFlag & ShapeFlags.ARRAY_CHILDREN) {
    mountChildren(
    children as VNodeArrayChildren,
    container,
    anchor,
    parentComponent,
    parentSuspense,
    namespace,
    slotScopeIds,
    optimized,
    )
    }

    changes to:
 if (shapeFlag & ShapeFlags.ARRAY_CHILDREN) { 
   if(parentComponent.isCE){parentComponent.ce._teleportTarget=container}
   mountChildren( 
     children as VNodeArrayChildren, 
     container, 
     anchor, 
     parentComponent, 
     parentSuspense, 
     namespace, 
     slotScopeIds, 
     optimized, 
   ) 
 } 
const outlets = (this._teleportTarget||this).querySelectorAll('slot')

@edison1105 edison1105 changed the title fix(runtime-dom): the _parseSlots of customElement(shadowRoot:false) should executed correctly fix(runtime-dom): handle nested customElement mount w/ shadowRoot false Sep 9, 2024
@edison1105 edison1105 changed the title fix(runtime-dom): handle nested customElement mount w/ shadowRoot false fix(custom-element): handle nested customElement mount w/ shadowRoot false Sep 9, 2024
@edison1105 edison1105 added 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. scope: custom elements wait changes labels Sep 9, 2024
@linzhe141
Copy link
Contributor Author

It seems the problem related to Teleport has not been resolved. see Playground with this PR看来传送相关的问题还没有解决。请参阅带有此 PR 的 Playground To fix this problem, it needs to use the teleport target to query all slots.为了解决这个问题,需要使用传送目标来查询所有插槽。

Suggestion 建议

  • Pass the teleport target to the ce during the teleport mounting在传送安装期间将传送目标传递给ce

    if (shapeFlag & ShapeFlags.ARRAY_CHILDREN) {
    mountChildren(
    children as VNodeArrayChildren,
    container,
    anchor,
    parentComponent,
    parentSuspense,
    namespace,
    slotScopeIds,
    optimized,
    )
    }

    changes to: 更改为:

 if (shapeFlag & ShapeFlags.ARRAY_CHILDREN) { 
   if(parentComponent.isCE){parentComponent.ce._teleportTarget=container}
   mountChildren( 
     children as VNodeArrayChildren, 
     container, 
     anchor, 
     parentComponent, 
     parentSuspense, 
     namespace, 
     slotScopeIds, 
     optimized, 
   ) 
 } 
const outlets = (this._teleportTarget||this).querySelectorAll('slot')

Thanks for your suggestion!

@edison1105 edison1105 added ready to merge The PR is ready to be merged. and removed wait changes labels Sep 11, 2024
Copy link

netlify bot commented Sep 11, 2024

Deploy Preview for vue-next-template-explorer failed.

Name Link
🔨 Latest commit a5550ee
🔍 Latest deploy log https://app.netlify.com/sites/vue-next-template-explorer/deploys/66e134b1dae6220008ddf9b8

Copy link

netlify bot commented Sep 11, 2024

Deploy Preview for vue-sfc-playground failed.

Name Link
🔨 Latest commit 789c993
🔍 Latest deploy log https://app.netlify.com/sites/vue-sfc-playground/deploys/66e1355e457502000822957d

@edison1105
Copy link
Member

edison1105 commented Sep 11, 2024

@linzhe141
I made some tweaks to your PR, see 50dd486
the root cause of the first problem of #11851 is that the child component's connectedCallback is performed due to


but the child is not connected yet.
this commit also fix #11871

more detail see #11871 (comment)

@edison1105 edison1105 added ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. and removed 🔨 p3-minor-bug Priority 3: this fixes a bug, but is an edge case that only affects very specific usage. labels Sep 11, 2024
@linzhe141
Copy link
Contributor Author

@edison1105 THANKS!!!!!!!!

@palasjir
Copy link

@edison1105 incredible! I can confirm this on #11861 fixes the issues. While testing I've run into issue first described here and currently can reproduce it in playground i think this

this._instance!.ce = undefined

should to be adjusted with

if(this._instance) {
  this._instance.ce = undefined
}

since this._instance can now actually be null when component was not connected.

@yyx990803 yyx990803 merged commit f2d8019 into vuejs:main Sep 13, 2024
13 checks passed
@linzhe141 linzhe141 deleted the chore-1 branch September 19, 2024 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. ready to merge The PR is ready to be merged. scope: custom elements
Projects
None yet
4 participants