View Transition Animation Not Working for Async Components in MDX Files

double-thinker commented 3 months ago

Astro Info

Astro                    v4.12.2
Node                     v18.20.3
System                   Linux (x64)
Package Manager          unknown
Output                   static
Adapter                  none
Integrations             @astrojs/mdx

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Async components (with a module-level await) do not work with view transitions only if rendered within an MDX page. The same async component is correctly animated within a standard Astro page.

What's the expected result?

The animation should work regardless of whether it is used within MDX or not.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-p2qmkm?file=src%2Fpages%2Findex.astro

Participation

[X] I am willing to submit a pull request for this issue.

martrapp commented 3 months ago

Hi @double-thinker, what a cool reproduction!

I have to admit that I have no idea what's going on at the moment. It might take some time to analyze and fix this.

Thanks for the offer to submit a PR. Since you did, do you happen to have any idea what's causing this behavior yet?

martrapp commented 3 months ago

The HTML sources of the two iframes only differ in one item. The CSS for the view transition of the second paragraph is missing in the MDX case. The CSS gets generated for the Async.astro component and is added to the _metadata.extraHead of the SSRResult.

The special thing about the Async.astro component is that it yields to the MacrotaskQueue in the script part of the component.

While the generated CSS makes it from the _metadata to the <head> when the component is used on an Astro page, it gets lost for the MDX page.

Maybe there is a missing await or the <head> closed to early? Fixing this is beyond my Astro skill & understanding. ;-) I will ask for help.

double-thinker commented 3 months ago

Due to Astro's streaming, we only wait until the page's frontmatter has run before generating the element. Frontmatter of a component cannot add scripts or stylesheets because by the time it's run, the is already sent to the browser so that it can start loading assets.

This becomes an issue when a component uses the content collections API. The render() call adds styles and scripts relevant to the content entry to the element. Therefore, it must be used nowhere other than the routed page's frontmatter, as that would lead to missing assets:

double-thinker commented 3 months ago

Ok, I'm not 100% sure, but it seems like this issue is related to the problem described in https://github.com/withastro/roadmap/discussions/707.

All transition: props in an async component cannot add head CSS because it's already sent to the client. A workaround is to wrap the async element and move up the transition: properties to the parent:

<div transition:animation="slide">
   <AsyncElement />
</div>

See the workaround here: https://stackblitz.com/edit/github-p2qmkm-bhkwwb?file=src%2Fcomponents%2FExample.astro

bluwy commented 3 months ago

This one is a bit hard to fix. The gist of the problem goes like this:

In .astro rendering, we have a prepass rendering phase where we do a quick init and crawl for Astro components recursively and we can detect which Astro components need head rendering. After that prepass, we can explicit render the heads of those components ahead, and by that time, result._metadata.extraHead will have the VT styles appended, good to go until the actual head render call later.

In .mdx (or any non-Astro page), they don't have a prepass feature. The pages are rendered as is, which means any promises will not be awaited if it later contributes any head. If you try to move that higher up to emulate a prepass, it's not going to work because it will do the actual head render call earlier. A prepass phase would not do any head render calls.

Ultimately, it's hard to fix this. We could try to create a prepass for .mdx but I'm not sure how well that scales for non-Astro pages. We could try an easy patch where we append later-added extraHead (like this), but it's an ugly hack and will likely cause more bugs in the future.

Another example why streaming is a PITA 🫠 I'm not sure how to do a nice fix for this at the moment, a workaround on the user-side may be needed.

Ok, I'm not 100% sure, but it seems like this issue is related to the problem described in withastro/roadmap#707.

It's not directly related I think as that proposal is for fixing renders in Astro files.

double-thinker commented 3 months ago

If you try to move that higher up to emulate a prepass, it's not going to work because it will do the actual head render call earlier. A prepass phase would not do any head render calls.

However, the workaround that moves them up is actually working. It seems like an MDX with sync components has no problem putting VT's CSS in extraHead.

Ok, I'm not 100% sure, but it seems like this issue is related to the problem described in withastro/roadmap#707. It's not directly related I think as that proposal is for fixing renders in Astro files.

From my understanding, the solution proposed in #707 could help here. If any non-Astro component has export const addsToHead, then the server should wait until they are rendered to send the head. IMO this solution is understandable for users, and it could be generalized to almost any framework. The other solution also helps: Astro.waitFor(<Content />) will explicitly ask to wait until the render finishes before sending the head.

withastro / astro