Open danielweck opened 2 years ago
Mhmm, this appears to be the cause of bug #25
No, it's not.
So this was my doing, and honestly, was just the result of trial and error.
Without awaiting the reset, when pages that would share styles (say /foo
and /bar
would be routed to the same content), pages that render afterwards do not contain any styles that exist in the previous. So /bar
would be empty (besides the preflight).
Twind utilizes Node's async_hooks
, and it seemed to me like there was a bad type somewhere that results in this incorrectly being labeled as sync. AFAIK, awaiting something that's synchronous has no effect.
If there's a better solution I'd love to see it.
Edit: See below for better reproduction of the original issue
Without awaiting the reset,
but reset()
is not an async function / doesn't return a Promise, so how come?
As I said:
just the result of trial and error.
Truly, I don't know, I think it was a misplaced await
but it "worked", and seemingly continues to do so.
It very well could be some race condition that await
just happens to "solve". I'd love a better solution if there is one.
it "worked", and seemingly continues to do so
Probably a side effect of Node's async_hooks
, similar to why the Promise.resolve().then()
wrapper seems to be necessary in prerender. Weird.
Here's a fresh reproduction repo, as the instructions I wrote above kinda suck: https://github.com/rschristian/twind-empty-duplicate-page-bug
Post install, this patches @twind/wmr
's prerender so you don't have to. So install deps, run a build, and then notice that dist/bar/index.html
is lacking all styles, when it should match 100% to dist/foo/index.html
.
You can then add the await
back into the module (node_modules/@twind/wmr/prerender/prerender.js
) it it'll work, dist/bar/index.html
has styles.
I haven't yet been able to break this, though I agree it's probably some sort of side effect that I'd think will fall apart eventually.
Thank you for the repro repo Ryan.
Regarding await sheet.reset()
(or in my case, the lack of await
which seems to make sense as reset()
is not async
): in the early days of my implementation I remember noticing inconsistent "bleeding" of styles (i.e. beyond the expected preflight
CSS) from one prerendered page to the next, which didn't make sense at all, and more importantly which would by definition be an unreliable mechanism as visitors to my website wouldn't necessarily enter via the root index.html
and/or in the order of generated Preact routes at build time. If I remember correctly, I hit this bug and quickly realised I had to implement my own logic anyway, due to the following:
Because of my specific functional requirements (notably: true zero-runtime Twind, i.e. Twind only used at build time), I decided early-on not to use use-twind-with/wmr
. I am of course using the fundamental building block of Twind's stylesheet reset + repopulate at each WMR prerender pass.
My custom build system ensures that each prerendered SSR'd HTML file contains "critical" inline CSS sufficient for its own presentation, and that each such HTML file also references "secondary" external CSS for all other hydrated / CSR'd routes.
In order to completely remove Twind's runtime, I implemented additional pre- and post- WMR build steps that compute "critical" vs. "secondary" styles ... similar in spirit to Twind's own "extract CSS" utility, but not just based on static analysis of used classes, instead I use a Preact options.vnode
-based logic that interprets dynamic Twind rules / possible component style variants depending on state, etc.
The "critical" inline styles are prerendered / SSR'd inside the head
of any given static HTML file, and my build process also generates lower-priority / "secondary" external stylesheet links that provide CSS for the hydrated CSR'd pages of Preact WMR's isomorphic router (including Suspense
dynamically-loaded Lazy
components, I might add).
Crucially: there is no overlap / redundancy between "critical" and "secondary" stylesheet rules, in order to minimise footprint and network traffic. There is possibly a caveat with respect to the specificity of some CSS selectors, but so far I have not hit this edge case.
Anyway, long story short: I would have to look into the specifics of use-twind-with/wmr
in order to figure out why await sheet.reset()
seems to automagically fix the bug you describe, but my gut feeling is that this is an async_hook
side effect.
UPDATE: I forgot to mention that ; of course ; the Twind "zero runtime" approach only applies to my WMR production builds, not the development server which does everything dynamically :)
https://github.com/tw-in-js/use-twind-with/blob/d40071a0d91319d4a07ceb09b4a03e4f4d7c729d/packages/wmr/prerender.ts#L25
...but:
https://github.com/tw-in-js/twind/blob/15441649dcdfb97fba56e0cc8870f08485cfc1c6/src/server/index.ts#L73-L85