feat(ejs): render and attach shadowroot in the same tick. #161
Conversation
src/TemplateElement.js
Outdated
| * @param {PropertyUpdateOptions} options | ||
| */ | ||
| update(options) { | ||
| let firstRender = this._options.shadowRender && !this.shadowRoot; |
There was a problem hiding this comment.
@quarkus this check will now run for every update cycle. There can be A LOT of update cycles... Is this ok performance wise?
There was a problem hiding this comment.
i guess we should set a simble "attached" flag somewhere, but yes, guess it makes sense to keep it as simple as possible.
There was a problem hiding this comment.
Hey @eddyloewen ! I challenged the idea but AI suggests that there is no performance to gain.
both boolean and the falsy check will be rather fast:
Accessing this.shadowRoot is a direct property lookup on the object, which is one of the fastest operations in JavaScript. It is a fundamental part of the object model and is highly optimized by browser engines.
It also pointed out that adding more flags will probably introduce new bugs which is probably also true :D
There was a problem hiding this comment.
ps.: i also moved it to the render function. maybe it makes more sense in combination with the adoptStyleSheets call.
src/TemplateElement.js
Outdated
| if (firstRender) { | ||
| this.attachShadow({ mode: 'open' }); | ||
| // adopt stylesheets | ||
| this.adoptStyleSheets(); |
There was a problem hiding this comment.
@quarkus where does this come from? This call was not here before...
There was a problem hiding this comment.
That actually comes from the "StyledElements" connectedCallback which we extend from here.
Since i moved the "attach" call here i have to somehow "retrigger" the adoption after the shadowroot is attached. Otherwise the styles are adopted into nohwere ...
There was a problem hiding this comment.
Please check again @eddyloewen . I rearranged the code and also removed it from the StyledElements connected as it actually needs a root to make sense.
…n (initStyleAdoption) which makes things hopefully better to understand. also removed the code from StyledElements connected as it needs a shadowRoot anyways to actually make sense.
Currently we have an animation frame between connection of an element and the first render cycle.
I ran into a bug today while rendering a Shadow Element as we actually attach the ShadowRoot on connected but actually render not until the requested update is performed.
That leaves the element in a state where the light dome is kind of "detached" from the DOM.
That leads to eg. nested elements having NO offsetParent and no dimensions at all:
With this change i would like to keep the SR Attache and the first render in the same tick.
Is there any reason why we should not do it like this @eddyloewen !?