head icon indicating copy to clipboard operation
head copied to clipboard

orphaned style tags

Open theoephraim opened this issue 2 years ago • 5 comments

I'm using this module to temporarily add some extra global style - in this case disabling scrolling on the html body, which can be useful in some cases like when a modal or nav is being displayed on top of the main content.

For example, my header/nav component has something like this (written in script setup style):

const isMobileNavOpen = ref(false);
function toggleMobileNav() { isMobileNavOpen.value = !isMobileNavOpen.value; }

// add extra body style to lock scrolling on main content while mobile nav is open
useHead({
  style: [{
    children: computed(() => (isMobileNavOpen.value ? 'body { overflow: hidden; }' : '')),
  }],
});

This works and the style is added/removed as the nav is toggled, locking body scroll as it should.

The problem is that if the component is unloaded, it is not cleaning up after itself and removing the extra style tag. This happens for example if I navigate to a different route that uses a different layout that does not contain this nav component.

I'm able to fix it by adding a onBeforeUnmount handler like so:

onBeforeUnmount(() => {
  useHead({ style: [{ children: '' }] });
});

but to me this was unexpected. I would have thought that any component that uses useHead would clean up after itself before unloading.

Please advise if this is expected or a bug that needs addressing. If it is a bug I can try to dig in and get it fixed.

Thanks!

theoephraim avatar Dec 15 '21 23:12 theoephraim

not sure I understand your setup, but have you tried using a simple permanent CSS sibling selectors to handle this? creating and blanking a whole stylesheet and bothering the DOM as well seems too much of an effort :)

If the nav/modal opens, apply a "toggle" class to the html or body element (the scroll container) and let the CSS do the work using the styles you already have, but instead of adding and removing the whole style element, turn it into a permanent selector in your global app style sheet which (by definition) would be inactive if the toggle class in the head or body is not set. I presume the CSS to block the scrolling is only a couple of lines.

The router events could also make sure that "stop-scrolling" class is removed if you navigate to/from a different layout/route.

WebMechanic avatar Feb 19 '22 14:02 WebMechanic

There’s definitely a few different ways to do this. Regardless I think this was very unexpected behaviour. If useHead adds something to the page from a component, I would expect it to clean up after itself when that component unmounts.

theoephraim avatar Feb 19 '22 15:02 theoephraim

I would expect it to clean up after itself when that component unmounts.

hmm, dunno. I get your point, but how is it supposed to know what of the stuff your component added for whatever reason should remain and what not? and under what circumstances? Just because some component is unmounted doesn't mean the whole page (or head) should go with it - me thinks...

For your scenario it would indeed be useful to have it linked to the router events (or any other component's custom on-and-off events) in some way so it does indeed cleanup after itself -- or make the contents/attributes of these element "more" reactive.

I'm not yet using this, so I'm unfamiliar with the internals. I plan to add link elements and meta that should "persist", but it seems I might run into a similar problem like you but for different reasons :D -- that's why I checked this issue in the first place.

Have fun.

WebMechanic avatar Feb 19 '22 15:02 WebMechanic

Personally I would expect that anything added by useHead would get removed when the component that added it unmounts. If you wanted certain items to persist, you'd put them in your main app component or layout components that don't get unmounted (or are replaced with another instance when they do). Otherwise you'd have lots of orphaned tags and things would not behave deterministically. I'm assuming useHead already works this way and my orphaned style tag example is just a bug.

theoephraim avatar Feb 19 '22 20:02 theoephraim

I see. I am still a little ambivalent about the whole thing and who's responsible cleaning up their mess ;-)

I'm assuming useHead already works this way and my orphaned style tag example is just a bug.

I wouldn't be able to tell, but I guess I'll find out very soon. However, it'll indeed be the app / layout component that's responsible for adding these tags.

Have a good time.

WebMechanic avatar Feb 19 '22 20:02 WebMechanic

Confirming this is still an issue.

Seems like a reactivity problem with computed

harlan-zw avatar Sep 19 '22 04:09 harlan-zw