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

Remove <template> mechanism #148

Closed
dascritch opened this issue Apr 12, 2021 · 4 comments
Closed

Remove <template> mechanism #148

dascritch opened this issue Apr 12, 2021 · 4 comments

Comments

@dascritch
Copy link
Owner

dascritch commented Apr 12, 2021

It seemed a good idea, using a standard system.
But in fact,

  1. we wait for the DOM to be ready
  2. we populate a <template> by writing its innerHTML,
  3. Then, for each component, we read the <template> innerHTML to write the shadowRoot via innerHTML.

Even if it was supposed to be done like that per standards, as our webcomponent is not supposed to overcloog the hosting pages, or sharing with other tier code that need its template, and we may create a lot of bottlenecking operations...

Let's remove it, we will also remove the unuseful DOMContentLoaded stop, so the paint operations will occur really sooner.

@dascritch
Copy link
Owner Author

dascritch commented Apr 12, 2021

Results are completely explosive. On my website (one <cpu-audio> in the front page, one <cpu-controller> in header) :

metric before after
build size 48781 48596
LightHouse perf on cpu.pm 76 92
First Contentful Paint 1,1 s 1.0 s
Speed Index 1,6 s 1.2 s
Largest Contentful Paint 2,9 s 2.8 s
Time to Interactive 2,7 s 2.2 s
Total Blocking Time 800 ms 330ms
Cumulative Layout Shift 0,147 0.144

So, yes, 3 writes and 2 read via innerHTML is really costly... instead of two writes only

dascritch added a commit that referenced this issue Apr 12, 2021
@dascritch dascritch reopened this Apr 12, 2021
@dascritch
Copy link
Owner Author

Can do a bit better

@dascritch
Copy link
Owner Author

No , no async (async () =>{ /* window.custom... */ } , this is a wrong idea, as we need CpuControllerElement instancied after CpuAudioElement

@dascritch
Copy link
Owner Author

Another one for the screenshot :

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant