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.

Sign up for GitHub

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

Jump to bottom

Astro slots passed as Svelte 5 snippets don't statically render #10512

Closed
1 task done
c9r opened this issue Mar 21, 2024 · 9 comments · Fixed by #12328
Closed
1 task done

Astro slots passed as Svelte 5 snippets don't statically render #10512

c9r opened this issue Mar 21, 2024 · 9 comments · Fixed by #12328
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: svelte Related to Svelte (scope)

Comments

@c9r
Copy link

c9r commented Mar 21, 2024 •
edited
Loading

Astro Info

Astro                    v4.5.8
Node                     v21.7.1
System                   macOS (arm64)
Package Manager          pnpm
Output                   hybrid
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/mdx
                         @astrojs/svelte
                         @astrojs/sitemap

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

No response

Describe the Bug

When statically generating (or server-side rendering) a .astro page that contains a Svelte 5.0.0-next.80 component, static children of the Astro instantiation don't statically render as snippets in the Svelte component. Below is a minimal example, followed by a likely root cause, and a potential fix.

Distilled example

Given the following Svelte 5 component, in button.svelte:

<script>
  const { children } = $props();
</script>

<button>
  {@render children()}
</button>

included from an Astro component, page.astro:

---
import Button from "./button.svelte";
---

<Button>Click Me!</Button>

the following static HTML gets rendered (note the absence of a "Click Me!" child text node):

<!DOCTYPE html><!--ssr:0--><button><!--ssr:1--><!--ssr:1--></button><!--ssr:0-->

Root cause

Take a look at the JS output for the aforementioned button.svelte component (playground link):

 // App.svelte (Svelte v5.0.0-next.80)
 // Note: compiler output will change before 5.0 is released!
 import * as $ from "svelte/internal/server";

 export default function App($$payload, $$props) {
   $.push(true);

   const { children, ...attrs } = $$props;
   const anchor = $.create_anchor($$payload);

   $$payload.out += `<button${$.spread_attributes([attrs], true,  false, "")}>${anchor}`;
+  children($$payload);
   $$payload.out += `${anchor}</button>`;
   $.bind_props($$props, { children });
   $.pop();
 }

Note that, in the compiled Svelte 5 component above, the children snippet gets invoked as a side-effecting function with a $$payload parameter.

But if you look at server-v5.js, the bridged snippet functions created by renderToStaticMarkup simply return the interpolated Astro slots, rather than appending the rendered output to $$payload.out (as appears to be the expectation, from a quick poke around the Svelte 5 codebase). Because the return value of the snippet function is unused by the compiled version of the Svelte component (at least as of v5.0.0-next.80), the child content of the Astro instantiation of the Svelte component simply gets discarded.

server-v5.js#L25-28

async function renderToStaticMarkup(Component, props, slotted, metadata) {
   // ...
   let children = undefined;
   let $$slots = undefined;
   for (const [key, value] of Object.entries(slotted)) {
     if (key === 'default') {
+      children = tagSlotAsSnippet(() => `<${tagName}>${value}</${tagName}>`);
     } else {
       $$slots ??= {};
+      $$slots[key] = tagSlotAsSnippet(() => `<${tagName} name="${key}">${value}</${tagName}>`);
     }
   }
   // ...
}

Potential fix

Modifying the snippet functions generated by renderToStaticMarkup in server-v5.js to append their output to $$payload.out, instead of returning it, leads to proper static rendering of the above button example.

server-v5.js

async function renderToStaticMarkup(Component, props, slotted, metadata) {
   // ...
   let children = undefined;
   let $$slots = undefined;
   for (const [key, value] of Object.entries(slotted)) {
     if (key === 'default') {
-      children = tagSlotAsSnippet(() => `<${tagName}>${value}</${tagName}>`);
+      children = tagSlotAsSnippet(($$payload) => $$payload.out += `<${tagName}>${value}</${tagName}>`);
     } else {
       $$slots ??= {};
-      $$slots[key] = tagSlotAsSnippet(() => `<${tagName} name="${key}">${value}</${tagName}>`);
+      $$slots[key] = tagSlotAsSnippet(($$payload) => $$payload.out += `<${tagName} name="${key}">${value}</${tagName}>`);
     }
   }
   // ...
}

What's the expected result?

Svelte 5 components should statically render children passed to them from Astro component instantiations. Note that the expected output includes the "Click Me!" text node that is absent from the currently generated output.

expected output

<!DOCTYPE html><!--ssr:0--><button><!--ssr:1-->Click Me!<!--ssr:1--></button><!--ssr:0-->

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-rw1mr4

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Mar 21, 2024
@matthewp
Copy link
Contributor

@bluwy do we need to do something to suppose Svelte snippets?

@matthewp matthewp added the pkg: svelte Related to Svelte (scope) label Mar 23, 2024
@bluwy
Copy link
Member

bluwy commented Mar 24, 2024

Yeah the issue proposed solution seems fine to me. I'm pretty sure I've fixed this before (#9285) but Svelte might've changed the implementation again.

@matthewp matthewp added - P3: minor bug An edge case that only affects very specific usage (priority) and removed needs triage Issue needs to be triaged labels Mar 25, 2024
nridwan added a commit to nridwan/astrojs-svelte5-patch that referenced this issue May 10, 2024
@nridwan
fix: withastro/astro#10512
09946ba
@und3fined
Copy link

Svelte 5 in RC from Apr 30 2024 any merged for this fix?

@bluwy bluwy self-assigned this May 14, 2024
@Lofty-Brambles
Copy link

@bluwy - any ETA on this? 😅
Named slots don't work in Svelte 5 either, the same bug occurs

@RATIU5
Copy link

RATIU5 commented Oct 3, 2024

I am also running into the same issue.

@Arecsu
Copy link

Arecsu commented Nov 5, 2024

up this issue

@ematipico
Copy link
Member

PRs are welcome, and OP is willing to send a PR

@bluwy
Copy link
Member

bluwy commented Nov 6, 2024

This should be fixed by #12328

@bluwy bluwy removed their assignment Nov 6, 2024
@bluwy bluwy linked a pull request Nov 6, 2024 that will close this issue
Add support for Svelte 5 @render syntax #12328
Merged
@bluwy bluwy closed this as completed Nov 6, 2024
@bluwy
Copy link
Member

bluwy commented Nov 6, 2024

Fixed in @astrojs/svelte 5.7.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) pkg: svelte Related to Svelte (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add support for Svelte 5 @render syntax Robertsson/astro
8 participants
@matthewp @ematipico @c9r @und3fined @Arecsu @bluwy @Lofty-Brambles @RATIU5