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.

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

render method 'svelte/server' too inflexible for emails, and non-page content #14120

Open
scosman opened this issue Nov 2, 2024 · 11 comments
Open
Milestone

Comments

@scosman
Copy link

scosman commented Nov 2, 2024

Describe the problem

I wrote the Svelte template https://github.com/CriticalMoments/CMSaasStarter and we are migrating it to Svelte 5.

One thing we use .svelte files for email templates for our mailer. I know it's not the intended use of svelte, but it seems weird to add a second templating system in a svelte project.

We use one template for plaintext and one for html of each email. Both worked fine in svelte 4 using Component.render. Both now have issues in svelte 5.

  • plaintext emails are now injecting HTML comments like <!--[--> and <!----> which I need to strip manually
  • HTML emails are logging warnings of node_invalid_placement_ssr: '<html>' because the email templates include html tags.

Describe the proposed solution

Maybe a "raw" option on the new render method in 'svelte/server'? I'm open to any fix though.

If this is explicitly out of scope we could move to another template system, but again, would love to not have 2 in one project.

Importance

nice to have

@dummdidumm
Copy link
Member

Please provide a reproduction for the second point ("HTML emails")

@scosman
Copy link
Author

scosman commented Nov 3, 2024

[Edited with fixed links]

For sure:

Template: https://github.com/CriticalMoments/CMSaasStarter/blob/47ba79bd86a97c26606fe5de5057a79a56089280/src/lib/emails/welcome_email_html.svelte

Code that calls it: https://github.com/CriticalMoments/CMSaasStarter/blob/47ba79bd86a97c26606fe5de5057a79a56089280/src/lib/mailer.ts

logged error: node_invalid_placement_ssr: (src/lib/emails/welcome_email_html.svelte:18:0) needs a valid parent element

@paoloricciuti
Copy link
Member

For sure:

Template: https://github.com/CriticalMoments/CMSaasStarter/blob/svelte_5_evlim/src/lib/emails/welcome_email_html.svelte

Code that calls it: https://github.com/CriticalMoments/CMSaasStarter/blob/svelte_5_evlim/src/lib/mailer.ts

logged error: node_invalid_placement_ssr: `` (src/lib/emails/welcome_email_html.svelte:18:0) needs a valid parent element

This yields a 404

@paoloricciuti
Copy link
Member

paoloricciuti commented Nov 3, 2024

So i took a look at the linked PR...the problem is that you are trying to render the whole html and body in a svelte component and you are using head instead of svelte:head.

The solution is quite simple imho: just remove the wrapper html, use svelte:head instead of head and remove the body tag. After the render you can compose the result adding back html, head and body with basic string concatenation...something like this

const result = render(Component);
const email = `<html lang="en"><head>${result.head}</head><body style="...">${result.body}</body></html>`;

@dummdidumm
Copy link
Member

dummdidumm commented Nov 3, 2024

If that did work in Svelte 4, I think we should investigate making that work in Svelte 5, too. The given component is valid HTML, and if our validation thinks that it's invalid we need to adjust it to see "ok this seems to be a standalone thing" (maybe this needs to be an option on the render method? because in the common case you're not doing the full html document, only a part of it)

@dummdidumm dummdidumm added this to the 5.x milestone Nov 3, 2024
@scosman
Copy link
Author

scosman commented Nov 3, 2024

Sorry about the broken links, fixed those!

From reading this, sounds like you might fix HTML, but plaintext is out of scope (no extraneous <!----> added)? That's understandable for Svelte, just asking for planning purposes.

@paoloricciuti
Copy link
Member

Sorry about the broken links, fixed those!

From reading this, sounds like you might fix HTML, but plaintext is out of scope (no extraneous <!----> added)? That's understandable for Svelte, just asking for planning purposes.

Tbh I think we should add an hydratable false option for when you want to use svelte just as a templating language.

@paoloricciuti
Copy link
Member

@scosman can you provide the whole warning that you were getting?

@paoloricciuti
Copy link
Member

@scosman can you provide the whole warning that you were getting?

Ok i was able to figure it out by myself...we could probably prevent the warning on render...however the warning is actually right: we use template to generate the html elements but with template html, body and head are simply discarder and that messes heavily with the traversing logic of svelte...so while it's unlikely that you will mount a svelte component that has a full html structure do we really want to prevent a warning that would actually be useful if by error you paste the whole html structure in a template that will need to be mounted on the client too?

@dummdidumm wdyt?

@scosman
Copy link
Author

scosman commented Nov 4, 2024

Looks like you already found it, but full error here:

node_invalid_placement_ssr: `<html>` (src/lib/emails/welcome_email_html.svelte:18:0) needs a valid parent element

This can cause content to shift around as the browser repairs the HTML, and will likely result in a `hydration_mismatch` warning.

FWIW I think it's a good warning, but there should be a flag to disable when someone knowingly wants to do it (hydrate=false like you suggested).

@ocluf
Copy link

ocluf commented Nov 5, 2024

Sorry about the broken links, fixed those!
From reading this, sounds like you might fix HTML, but plaintext is out of scope (no extraneous <!----> added)? That's understandable for Svelte, just asking for planning purposes.

Tbh I think we should add an hydratable false option for when you want to use svelte just as a templating language.

Agree with this. As an example I am currently trying to make a svelte 5 port of react email and running into the same issue. Using this ugly hack to get around it at the moment.

const originalConsoleError = console.error;
console.error = (message, ...args) => {
// Suppress ONLY the specific error message related to node_invalid_placement_ssr
if (typeof message === 'string' && message.includes('node_invalid_placement_ssr')) {
	return; // Ignore this specific error, do nothing
}
// For any other error messages, call the original console.error
originalConsoleError(message, ...args);
};
const { body, head } = render(component, { props });
console.error = originalConsoleError;

dummdidumm added a commit that referenced this issue Nov 26, 2024
This reverts #13255 / #13158, and helps with the validation error part of #14120

The revert is justified because people should be free to render a component in isolation and then append it somewhere where they know the rendered output is valid. If someone renders a tag at the top level that is invalid without a certain parent (which itself is an edge case), then the likelihood that this is done on purpose is much higher than that it's done by accident.
dummdidumm added a commit that referenced this issue Nov 26, 2024
This reverts #13255 / #13158, and helps with the validation error part of #14120

If you would have a component like this...
```svelte
<td>hi there</td>
```
...and then render it on the server via our `render` function like this:
```js
const result = render(Main);
```
...then right now you get an error saying that `td` is not valid at this position. But that doesn't seem right, because we should give people the benefit of the doubt: It may very well be that someone renders such a component and then correctly puts it into a `tr` tag themselves on the server (another example is rendering a full html document like in #14120).

All the other validation where there's a known parent (X not valid inside Y) is untouched by this.

Doing this as cleanup prior to tackling #13331
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants