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

Purpose/Performance thoughts #13

Open
bkardell opened this issue Feb 15, 2024 · 3 comments
Open

Purpose/Performance thoughts #13

bkardell opened this issue Feb 15, 2024 · 3 comments

Comments

@bkardell
Copy link
Owner

Currently, half-light makes a few important decisions on how it works....

  1. It uses a @media rule to keep everything valid and useable from CSSOM. This saves re-fetching or re-parsing (as well as the code it would take to do that which is huge) because everything is valid.

  2. It uses requestAnimationFrame to wait a beat for the body to become available before making sure rules come from the head and then collecting them all into the sets they might match. More or less this shouldn't fire until very shortly after it is and it could, in theory paint something

  3. Right after 2, it attaches a mutation observer to the head. Again, the head should be closed/done - but if someone (rarely?) manipulates a style or link tag in the head after, it will work. For example, if you open devtools and edit rules in a <style> that should actually work as you expect.

  4. Creates a proxy(conceptually)/aspect around .attachShadow() which means that it can be pretty performant and work for whenever/however shadows are attached.... As long as it's done by calling that method. DSD does not, and so it doesn't work with DSD.

Perhaps #3 is debateable. Someone mentioned that that gave them some pause about adopting it. It's not really fundamentally necessary for many/most use cases I guess. It sure makes it nice when you want to play around with dev tools or a code pen or something though.

Is #4 the right tradeoff? @knowler was mentioning (I think) that you could walk the tree and check for shadow roots, and that would support DSD. I guess we could just provide a method you could call manually when ready and it would happen late, just once and that would support DSD? It would be hella efficient I guess, but have bad FOUC and wouldn't handle anything live? I'm not sure that's a set of tradeoffs I would make myself though -- another option could be to just use a mutation observer on body instead - it's less efficient maybe by a lot, but almost all mutations just bail out because you're just checking for a shadow root and this is a thing a lot of polyfills do.. and it would work with DSD, not late.

So... What are we going for here? I'm debating maybe even just making a few flavors of the script because none of them are very complicated... I could use more thoughts before I make any updates to these though.

@sorvell
Copy link

sorvell commented Feb 15, 2024

Here's my $0.02...

  1. Using @media is good, but walking style rules imposes a potentially notable cost (average CSS size is ~75KB). There should be an opt-out (or in?) attribute data-no-half-light or something. I think a very common use case is to just pull in 1 or a few entire stylesheets (e.g. a reset or library like bootstrap) and specify the media attribute on it. If that's done, it shouldn't be processed (not sure if it is now), and there might be other stylesheets intended for the page only that it would be nice to prevent processing.
  2. I agree that preventing FOUC is important, and I believe waiting for requestAnimationFrame be fine to accomplish this. Theoretically, I guess the parser could yield and paint before the head is done, but that seems rare enough not to bother with.
  3. In think this should be optional. The concern I have isn't the mutation observer but the Set of shadowRoots maintained. That's a memory leak since it'll maintain references to elements removed from the DOM. Dealing with the removal via disconnection would be possible, but imo, it's not worth it and would require either an intrusive mutation observer or a base class implementing disconnectedCallback. I don't think this library should be coupled to custom elements if possible since Shadow DOM is not.
  4. I like the attachShadow override. Walking nodes is problematic for the general case as you mention, and an opt-in API is cumbersome. I do think it might be worth adding DSD support that just does a 1 time walk only. DSD is really not dynamically created after the page is complete.

Re: A few flavors... Personally, I wouldn't bother with this. Aside from being harder to maintain, it's cumbersome to choose and switch. I think the "live styles" option should either just be removed or I'd favor just having a data-observe attribute on the script that you get via document.currentScript.

@bkardell
Copy link
Owner Author

@sorvell does #1 differ from https://github.com/bkardell/shadow-boxing? Or do you just prefer that one? Like, if we switch #1, what is left of half-light and how is it not just shadow-boxing?

re: #4 ... This seems pretty reasonable I guess.. When? On DOMContentLoaded?

@sorvell
Copy link

sorvell commented Feb 26, 2024

what is left of half-light and how is it not just shadow-boxing?

Things I value about this approach:

  • can consume either an entire stylesheet or a subset of rules within a stylesheet
  • provides a selector to specify what scopes to which the styling should apply

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

2 participants