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

🐛 BUG: Collection of <head> issues as of v0.21.10 #2128

Closed
philipp-tailor opened this issue Dec 5, 2021 · 11 comments
Closed

🐛 BUG: Collection of <head> issues as of v0.21.10 #2128

philipp-tailor opened this issue Dec 5, 2021 · 11 comments
Assignees
Milestone

Comments

@philipp-tailor
Copy link

philipp-tailor commented Dec 5, 2021

What version of astro are you using?

0.21.10

Describe the Bug

I am aware about the head RFC, and the linked GitHub issues. I still felt that a complete overview over current behaviors is missing for people dealing with <head/> related issues.
These variations below are my most relevant attempts to create a structure to manage composite, dynamic head content in a project with nested layouts. Some of the variations - in my opinion - should work. Others of them I didn't expect to work, and they indeed did not. But that none of them work is a deal breaker for my use-cases. Behaviors changed in a breaking manner from v0.20.x to v0.21.x without mention in the changelog, but it appeared to me that they even changed between different v0.21.x patch versions (I might be wrong about that). The variations of unexpected / non-working behaviors described below reflect the state as of v0.21.10.
With this issue I hope to raise awareness for (potential) users of astro, and show up variations of possible cases the RFC, astro, and the test suite could cover. This overview is probably incomplete - feel free to document more behaviors as you detect them below.

1. Head cant't be complemented with content passed in fragment to slot in head

<!-- components/BaseHead.astro -->
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<meta name="color-scheme" content="light dark">
<!--layouts/BaseLayout.astro-->
<head>
	<BaseHead />
	<slot name="head"/>
</head>
<!-- layouts/TopPageLayout.astro-->
<BaseLayout>
	<Fragment slot="head">
		<meta property="og:type" content="article" />
		<link rel="stylesheet" href="/article.css" />
	</Fragment>
</BaseLayout>

Expected:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<meta property="og:type" content="article" />
	<link rel="stylesheet" href="/article.css" />
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>
	<meta property="og:type" content="article" />
	<link rel="stylesheet" href="/article.css" />
</body>

2. Head cant't be complemented with head element passed to slot in head

This is a variation of 1., which doesn't make much sense if astro is to mirror HTML, since there can only be one head in the HTML output. However, I hoped it might lead the compiler to detect that the content passed to the head slot shall be attached to the head. It doesn't, and instead behaves the same as 1..

<!-- components/BaseHead.astro -->
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<meta name="color-scheme" content="light dark">
<!--layouts/BaseLayout.astro-->
<head>
	<BaseHead />
	<slot name="head"/>
</head>
<!-- layouts/TopPageLayout.astro-->
<BaseLayout>
	<head slot="head">
		<meta property="og:type" content="article" />
		<link rel="stylesheet" href="/article.css" />
	</head>
</BaseLayout>

Expected:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<meta property="og:type" content="article" />
	<link rel="stylesheet" href="/article.css" />
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>
	<meta property="og:type" content="article" />
	<link rel="stylesheet" href="/article.css" />
</body>

3. Head content is rendered in body when head is set as default slot content

I am not sure if this is a variation of this issue?

<!-- components/BaseHead.astro -->
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<meta name="color-scheme" content="light dark">
<!--layouts/BaseLayout.astro-->
<slot name="head">
	<head>
		<BaseHead />
	</head>
</slot>
<!-- layouts/TopPageLayout.astro-->
<BaseLayout/>

Expected:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head></head>
<body>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</body>

4. Head can't be set when passed as slot

This expands upon 3., and I include it to show what I expected to happen if content is passed to the head slot.

<!-- components/BaseHead.astro -->
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<meta name="color-scheme" content="light dark">
<!--layouts/BaseLayout.astro-->
<slot name="head">
	<head>
		<BaseHead />
	</head>
</slot>
<!-- layouts/TopPageLayout.astro-->
<BaseLayout>
	<head slot="head">
		<BaseHead />
		<meta property="og:type" content="article" />
	</head>
</BaseLayout>

Expected:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<meta property="og:type" content="article" />
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head></head>
<body>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<meta property="og:type" content="article" />
</body>

5. Multiple heads rendered in different layouts are not compositing

Again, a case that does not make sense from HTML perspective, but in depsparation, I tried whether it would work nevertheless.

<!-- components/BaseHead.astro -->
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width,initial-scale=1.0" />
<meta name="color-scheme" content="light dark">
<!--layouts/BaseLayout.astro-->
<head>
	<BaseHead />
</head>
<!-- layouts/TopPageLayout.astro-->
<BaseLayout>
	<head>
		<meta property="og:type" content="article" />
	</head>
</BaseLayout>

Expected:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<meta property="og:type" content="article" />
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/TopPageLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>
	<meta property="og:type" content="article" />
</body>

6. Head can't be included only by rendering component rendering head

This one has already been reported. The code below does not use nested layouts, and does not have dynamic parts to it. That this simple case does not work, excludes all application structures where different layouts render <BaseHead.astro with dynamic head content passed to the slots.

<!-- components/BaseHead.astro -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<slot/>
</head>
<!--layouts/BaseLayout.astro-->
<BaseHead />

Expected:

<!-- Build output of page using `layouts/BaseLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/BaseLayout.astro` -->
<body>...</body>

7. Head set from svelte components hydrated on client load is not part of build output

Another, advanced case that does not work, is including content from svelte components with <svelte:head> in the static build output. I assume that it's extremely difficult for astro to detect if a svelte component is always rendered and hydrated in a layout with client:load, and always limited to only being changed from the svelte component when its bundled JS is being executed?

<!-- components/BaseHead.svelte -->
<svelte:head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
	<slot/>
</svelte:head>
<!--layouts/BaseLayout.astro-->
<BaseHead client:load />

Expected:

<!-- Build output of page using `layouts/BaseLayout.astro` -->
<head>
	<meta charset="utf-8" />
	<meta name="viewport" content="width=device-width,initial-scale=1.0" />
	<meta name="color-scheme" content="light dark">
</head>
<body>...</body>

Actual:

<!-- Build output of page using `layouts/BaseLayout.astro` -->
<body>...</body>

The head then changes once BaseHead.svelte is hyrated on the client.

@natemoo-re
Copy link
Member

Thank you! This is a very thorough and helpful overview.

@FredKSchott
Copy link
Member

FredKSchott commented Dec 6, 2021

Thank you!! This will be super useful as we work on implementing withastro/roadmap#26. We can make sure that all of these are handled before we mark that work as done.

One thing that stands out as maybe not possible in the new head RFC is <head slot="head">. This could be too difficult to support if we want the page renderer to warn if two <head> elements appear. Added this as a callout to the RFC here: https://github.com/withastro/rfcs/blob/FredKSchott-patch-2/active-rfcs/0000-finalize-head-body.md#head-injection-changes

@philipp-tailor
Copy link
Author

philipp-tailor commented Dec 7, 2021

I'm very glad this helps!

As briefly touched above, I did not expect 2. and 5. to work, and if I read the RFC correctly, it also won't, which makes sense.
If I understand this line correctly, case 7. with Svelte is also not intended to work. Which I understand, but have ambiguous feelings about, because it feels like it taints the "bring our own framework for any islands, and it'll work" pitch ever so slightly. Anyway, astro isn't 1.0, and this is not so pressing.

One thing that stands out as maybe not possible in the new head RFC is <head slot="head">

I don't know astro's internals, but would hope this would work. Since <head> can be rendered in an .astro template file, <head> should ideally have the same limitations as any other element rendered by the template. Exceptions always require docs.
Since you are bringing up that this might not work, I assume that the "linting" step of the page renderer is currently not running as a last step after the full page output has been assembled, but before that?

I want to mention that of course I'd be happy to help testing, and documenting the post-RFC <head> behaviors. Generally speaking I could also help implement the RFC, but have limited time resources and so far 0 understanding of astro internals, so I'd need some good pointers.

@philipp-tailor
Copy link
Author

philipp-tailor commented Dec 7, 2021

Actually, if I argue that <head> should behave like any other element in .astro templates, one could argue that we could allow any component to include <head>, and the compiler could merge all contents contained. In that scenario, <head> would not work like in HTML, but more like a globally available slot. The compiler anyway is already computing the content of <head> to include all the imports etc.
I found this to feel very frictionless and surprisingly natural when working with svelte. Doing so would remove the need for my more complex attempts to pass around dynamic head things via slots.
The trade-off is that it would be a bit unexpected, since it's a unlike standard HTML behavior. On the other hand, it's an optional feature, and <head> could still be used exactly like in plain HTML. And the whole point of a framework is to make recurring tasks easier, which astro also already does for things like RSS, sitemap generation, having markdown baked in, etc.

@philipp-tailor
Copy link
Author

Please let me know if I should move these comments to the RFC.

@FredKSchott
Copy link
Member

@philipp-tailor would love your help implementing if you can make time! Would love to pair you with someone on the core team to help tackle together.

Would love your comments on the RFC itself, we'll actually be doing our weekly RFC call on Discord ~2 hours from now, and you could leave feedback there as well.

<head> collapsing/injection across multiple components was discussed and actually was originally a part of this RFC! But, it was ultimately removed from the RFC, since it was technically a new feature that could be built on top of this RFC and didn't need to block the larger "finalize our API" discussion.

@philipp-tailor
Copy link
Author

@FredKSchott Thanks, I joined the call, and the RFC makes sense given that the head coalescing has been removed from scope on purpose.

I'd be open to helping to implement, if you point me someone's way (or vice-versa).

@natemoo-re
Copy link
Member

Sorry that this has dragged a bit, everyone!

Just to give an update, this is on track to be fixed in withastro/compiler#267 and the subsequent 0.24.x release of astro.

@philipp-tailor
Copy link
Author

Thanks so much @natemoo-re! I assume the "final" expected head behaviors will be in the documentation of 0.24.x, and can be read in the meantime in the RFC withastro/roadmap#26?

@natemoo-re
Copy link
Member

This class of issue should definitely be fixed in 0.24.x. We will work on getting best practices documented, but the basic gist of this change is that any special rules about where <head> can/can't go have been removed.

Some of these examples expect some sort of "head collapsing" behavior, which is still not something that Astro supports. But you should be able to use <slot name="head"> inside of <head> and pass content in via <Fragment slot="head">.

<svelte:head> content is something else that we should fix. I've opened another ticket for that.

See https://stackblitz.com/edit/github-accdiv-pum797?file=src%2Fpages%2Findex.astro and https://stackblitz.com/edit/github-accdiv-nyt1bq?file=src%2Flayouts%2FBaseLayout.astro

@natemoo-re
Copy link
Member

Per my comment above, most of these should be fixed in the newly released [email protected]!

Please feel free to open a new issue for any problems you run into.

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