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): Use asynchronous custom element nesting to avoid errors #9351

Open
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

baiwusanyu-c
Copy link
Member

@baiwusanyu-c baiwusanyu-c commented Oct 7, 2023

close: #9341
close: #8127

@github-actions
Copy link

github-actions bot commented Oct 7, 2023

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 89.8 kB (+339 B) 34.1 kB (+91 B) 30.8 kB (+76 B)
vue.global.prod.js 147 kB (+339 B) 53.4 kB (+94 B) 47.7 kB (+73 B)

Usages

Name Size Gzip Brotli
createApp 49.8 kB 19.5 kB 17.8 kB
createSSRApp 53.1 kB 20.8 kB 19 kB
defineCustomElement 52.4 kB (+339 B) 20.4 kB (+89 B) 18.6 kB (+70 B)
overall 63.3 kB 24.4 kB 22.3 kB

@baiwusanyu-c
Copy link
Member Author

/ecosystem-ci run

@vue-bot
Copy link

vue-bot commented Oct 11, 2023

📝 Ran ecosystem CI: Open

suite result latest scheduled
language-tools success success
nuxt success success
pinia success success
quasar failure failure
router success success
test-utils success success
vant success success
vite-plugin-vue success success
vitepress success success
vue-i18n success success
vue-macros failure failure
vuetify success success
vueuse success success
vue-simple-compiler success success

@edison1105
Copy link
Member

also fix #8127

@yashha
Copy link

yashha commented Dec 14, 2023

@baiwusanyu-c
Found an issue with mounting custom elements within a vue subcomponent, that is rendered conditionally.
https://stackblitz.com/edit/vitejs-vite-eyrmms?file=package.json

@baiwusanyu-c
Copy link
Member Author

I will take the time to check the issue

Copy link

codspeed-hq bot commented Dec 15, 2023

CodSpeed Performance Report

Merging #9351 will improve performances by 73.58%

Comparing baiwusanyu-c:bwsy/fix/CENestedAsync (c02daeb) with main (04d2c05)

Summary

⚡ 1 improvements
✅ 52 untouched benchmarks

Benchmarks breakdown

Benchmark main baiwusanyu-c:bwsy/fix/CENestedAsync Change
write reactive obj, read 1000 computeds 68.6 ms 39.5 ms +73.58%

@baiwusanyu-c
Copy link
Member Author

@baiwusanyu-c Found an issue with mounting custom elements within a vue subcomponent, that is rendered conditionally. https://stackblitz.com/edit/vitejs-vite-eyrmms?file=package.json

Is your problem that <h1>Custom Component</h1> is not rendering after clicking the button?

@yashha
Copy link

yashha commented Dec 15, 2023

@baiwusanyu-c Yes, the shadowroot of the custom element is staying empty, without the patch it is working

@baiwusanyu-c
Copy link
Member Author

try again @yashha

@yashha
Copy link

yashha commented Dec 15, 2023

@baiwusanyu-c
In the minimal project in stack blitz it is working now properly
https://stackblitz.com/edit/vitejs-vite-v7r9rs?file=patches%2F%40vue%2Bruntime-dom%2B3.3.7.patch

But in our project we have now bad performance in dev mode, we run in some kind of loop, I don't know whats the cause.

I get several of these:

Uncaught (in promise) out of memory 4
uncaught exception: out of memory
Uncaught out of memory

@yashha
Copy link

yashha commented Dec 15, 2023

It was something on our side, we had a component without template tags only script tags, that caused the issue.

@baiwusanyu-c
Copy link
Member Author

It should be that references cause memory usage. I will consider how to solve this problem.

@SavkaTaras
Copy link

SavkaTaras commented Feb 5, 2024

Hello @baiwusanyu-c and @edison1105 ,
I hope you are doing well.

Could you please review this and merge if possible? We ran into this issue as well, when wc imports wc within the slot.

Thank you,
Taras Savka

@SavkaTaras
Copy link

Hello @baiwusanyu-c @edison1105 @yyx990803 @LinusBorg ,
I hope you guys are doing well.

I am so sorry for bothering you all, but could you please take a look to see if this could be merged, and if so, please merge it? We are waiting on this PR.

Thank you again for all your great work,
Taras Savka

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Review
5 participants