@next: Calling setup() multiple times leads to orphaned style tag

[rschristian]

Edit: Sorry in advance for this wall of text.

In short, style tags can be become orphans after calling setup() multiple times, leading to flashes of unstyled content. This results in a back-and-forth workflow of styles flashing every other refresh.

Description / Walkthrough

1. Initial page load
   1. On initial load, without a sheet passed as a param, setup will call getSheet()
      

      twind/packages/twind/src/runtime.ts

      Lines 96 to 100 in d7cd293

      	export function setup<Theme extends BaseTheme = BaseTheme, SheetTarget = unknown>(
      	config: TwindConfig<any> | TwindUserConfig<any> = {},
      	sheet: Sheet<SheetTarget> = getSheet() as unknown as Sheet<SheetTarget>,
      	target?: HTMLElement,
      	): Twind<Theme, SheetTarget> {

   2. This will (can? not sure about all the branches here) lead to a style tag being created in the head of the document, as none exists at this time:
      

      twind/packages/twind/src/sheets.ts

      Lines 4 to 14 in d7cd293

      	function getStyleElement(element?: Element | null | false): HTMLStyleElement {
      	let style = element || document.querySelector('style[data-twind]')
      	if (!style || style.tagName != 'STYLE') {
      	style = document.createElement('style')
      	;(style as HTMLElement).dataset.twind = ''
      	document.head.prepend(style)
      	}
      	return style as HTMLStyleElement
      	}

   3. setup, after sorting its params, will firstly destroy the active sheet instance. None exists at this time, so nothing happens
      

      twind/packages/twind/src/runtime.ts

      Line 101 in d7cd293

      active?.destroy()

   4. The styles get loaded into the previously created style tag and we have styles on the page
      

      twind/packages/twind/src/runtime.ts

      Line 103 in d7cd293

      active = observe(twind(config as TwindUserConfig, sheet), target)

2. HMR/Calls setup again
   1. Still no sheet, so getSheet() gets called again
   2. This time it does find an existing style tag, so it returns a reference to that rather than creating a new one.
   3. active now exists, so the .destroy() occurs. This removes the style tag from the document, though the reference to this tag still exists (the sheet param).
   4. The styles get loaded in the now orphaned sheet.
   5. User is left without any styles.
3. HMR/Calls setup a third time
   1. getSheet()
   2. No existing style tag, so creates a new one and returns it.
   3. active does exist, but its sheet instance is the orphan. This gets removed again (nothing functionally happens as it's already been removed)
   4. The new style tag is populated with styles.

This then goes on & on, which each odd-numbered refresh resulting in a page with styles, even-numbered is blank.

Additional

I toyed around with enforcing that dom create a new style tag and that "works", but that might not work at scale. Didn't actually run the tests with it, so not sure.

twind/packages/twind/src/sheets.ts

Lines 71 to 72 in d7cd293

	export function dom(element?: Element | null | false): Sheet<HTMLStyleElement> {
	const target = getStyleElement(element)

There's also that problem in that just by trying to destroy the active sheet, Twind doesn't have keep a reference to the Node, but potentially searches for it. We can't simply move the getSheet() after the .destroy() because of this. Hmm, I actually don't understand what's happening here. Might've written this too soon.

The actual user code is this block:

https://github.com/rschristian/twind-wmr/blob/88d5e648cebce03428f7c4a81560dfda87b9b249/src/index.ts#L14-L20

Is there something else I should be using in this situation? The docs indicate that setup can be called multiple times, though this seems like it can (will?) lead to stale references like this.

Thanks as always for this awesome lib!

[sastan]

It may take some time to dig through this.

[rschristian]

Sorry if my description is awful, certainly let me know and I can try to rephrase if it didn't make sense.

Can't believe I also forgot a reproduction. Here's one: https://github.com/rschristian/twind-wmr-setup-bug. Simply run the dev server (npm run start) and save public/index.js a few times to trigger the HMR. It'll flash between Twind styles being applied and then disappearing (due to the style tag being removed and not replaced).

I am still trying to look into it myself, though there's a few bits that I don't understand.

Edit: Ah, I'm a fool. A fix I had tried and was sure would work, but didn't, was failing due to the implementation details of install which I didn't notice. That'll require a bit more thought. I do have a fix(-ish) for setup however, so I'll open a draft PR for that.

[sastan]

should be fixed by 0e2aa5c in the next release

[rschristian]

@sastan Manually patched and can confirm, working beautifully! Thanks!

I had ended up doing something similar, though forgot to update the PR. Sorry about that.