Finalize <head>, <body>, <html> by FredKSchott · Pull Request #26 · withastro/roadmap

FredKSchott · 2021-12-01T04:57:01Z

Start Date: 2021-11-30
Status: Accepted

Summary

Finalize behavior around <head>, <body>, <html>, and <!DOCTYPE html> elements in Astro component templates.

Links

Full Rendered Proposal

active-rfcs/0000-finalize-head-body.md

matthewp · 2021-12-01T22:24:49Z

active-rfcs/0000-finalize-head-body.md

+  - an `<html>` tag is always included in the final output
+- `<!DOCTYPE html>`
+  - completely ignored in a component template
+  - a `<!DOCTYPE html>` tag is always included in the final output


I'm fine with this personally, we should search discord just to be sure there aren't people out there wanting to use older doctypes though.

matthewp · 2021-12-01T22:32:02Z

Couple of notes:

This is missing use-cases. There are some known use-case from within SPA frameworks, but are those use-cases relevant in a framework where you have control over the entire page template? A comparison of the approaches would be helpful here.
One drawback that is not listed here is streaming SSR. This probably prevents it. If we have to wait for all components to render before knowing what to put into the head, we can't stream out the HTML.
- This is perhaps not true if there was a static requirement place on the component-level head usage (in which case it could be extracted at compile time), but this RFC doesn't talk about any restrictions.

jonathantneal

I can dig it. Left some comments.

active-rfcs/0000-finalize-head-body.md

FredKSchott · 2021-12-02T00:03:01Z

@matthewp: This is missing use-cases. There are some known use-case from within SPA frameworks, but are those use-cases relevant in a framework where you have control over the entire page template? A comparison of the approaches would be helpful here.

Let me see if I can improve the "motivation" section based on your other item of feedback. This section is meant to answer this question.

@matthewp: One drawback that is not listed here is streaming SSR. This probably prevents it. If we have to wait for all components to render before knowing what to put into the head, we can't stream out the HTML.

I believe that this is already not supported based on how layouts work, but I agree that this is making that lack of support explicit. I'll add a note to the Drawbacks section.

active-rfcs/0000-finalize-head-body.md

FredKSchott · 2021-12-02T20:26:11Z

@matthewp @jonathantneal thank you for the feedback and comments on this RFC, and @natemoo-re for giving feedback in person.

Based on feedback (and a better understanding of how this all works in the current codebase) I took another pass at this and really re-scoped the RFC:

<head> injection/collapsing has been removed as out-of-scope, to be tackled in a separate RFC.
No longer talking about adding or removing head/body/html elements on the page automatically, and instead deferring to the component author.

The RFC is now only focused on simplifying things in the codebase and committing to a more straightforward render behavior. Hopefully the smaller scope makes this easier to review. PTAL when you can!

Note: previous RFC draft still available here

active-rfcs/0000-finalize-head-body.md

matthewp · 2021-12-02T21:45:07Z

active-rfcs/0000-finalize-head-body.md

+- `<html>`, `<body>`, `<head>`
+  - compiler: will be left as-is in the component template.
+  - runtime: will be left as-is in final page HTML output.
+  - runtime: may warn if duplicate `html`, `body`, and `head` tags are included in final page HTML output.


this may be difficult, remembering that the runtime only see strings of HTML, so not easy to detect multiple tags.

I can it make more clear that this isn't required, and is just a suggestion of something that is not impossible/blocked by this RFC.

Co-authored-by: Jonathan Neal <jonathantneal@hotmail.com>

matthewp · 2021-12-02T21:45:30Z

active-rfcs/0000-finalize-head-body.md

+
+## Compiler Changes
+
+- `src/pages` and `src/layouts` will no longer be parsed or rendered differently from other components


👍 agree on this

Co-authored-by: Jonathan Neal <jonathantneal@hotmail.com>

active-rfcs/0000-finalize-head-body.md

jonathantneal

I have one lingering concern left. Looking for clarity regarding <!doctype html>.

FredKSchott · 2021-12-03T04:57:14Z

Resolved in convo above!

active-rfcs/0000-finalize-head-body.md

RafidMuhymin · 2021-12-07T12:12:42Z

I am mentioning a use case for hoisting <link> or <meta> tags to the <head> of the page.

Most of the images on a webpage should be lazily loaded. But some images like banners, hero images, or logos (it should be inline though) should be eagerly loaded. To do this, one should not only tell the browser to eagerly load the image using loading="eager" but he should also preload the image. For this, users can pass a format using the preload prop to the <Image> component to eagerly preload the image.

But when I was implementing this feature I found that unlike v0.20, <link>, <meta> or other tags that should be in the <head> of the page aren't hoisted.

So, not only the <Image> component is affected but any third-party package or even a user wants to implement a similar feature can't do this.

matthewp · 2021-12-07T12:59:59Z

active-rfcs/0000-finalize-head-body.md

+
+- runtime: will remove current post-build head injection, which involves a fragile `'</head>'` string find-and-replace.
+- compiler: will add a new `head: string` (or: `injectHead(): string`) property to the compiler transform options, which will inject the given HTML string into the bottom of a `<head>` element, if one is return by the compiler.
+- runtime: will provide this value head injection value to the compiler, and throw an exception if not used/called exactly once during a page render.


matthewp · 2021-12-07T13:03:55Z

active-rfcs/0000-finalize-head-body.md

+## Head Injection Changes
+
+- runtime: will remove current post-build head injection, which involves a fragile `'</head>'` string find-and-replace.
+- compiler: will add a new `head: string` (or: `injectHead(): string`) property to the compiler transform options, which will inject the given HTML string into the bottom of a `<head>` element, if one is return by the compiler.


I like injectHead() rather than injectStyles() / injectLinks() because it gives the runtime the freedom to inject anything it needs into the bottom of the head, in whatever order it decides is best.

I don't think this should be part of the compiler options though. I would expect this to be part of the runtime code. Either part of the $$result object or an import, but I think this is an implementation detail that doesn't need to be spelled out in the RFC.

ah, I thought that we needed it to be in the compiler since transform() returns a final string, but maybe I misunderstood. Agreed, happy to finalize this in the implementation since this RFC is mainly focused on expected behavior.

FredKSchott · 2021-12-07T19:26:53Z

RFC Notes:

document this behavior in the docs site and tests
Question: can we only add a <!DOCTYPE html> if an <html> element is detected?
- we need a head already, so "html fragment" is not currently a supported usecase by this RFC
Status: Accepted with consensus on RFC call

matthewp · 2021-12-11T17:41:15Z

@RafidMuhymin Thank you, this is great feedback and something we should figure out a way to support. If you'd like, we can start a new Discussion in this repo to talk about that use-case more.

RafidMuhymin · 2021-12-15T19:16:52Z

@matthewp Yeah, I have no problem doing that if you tell me what's the way of starting a discussion in this repo

FredKSchott · 2021-12-15T19:43:39Z

yes please! Happy to revisit that part of this, it came up in the convo and the balance we need to strike is supporting users who are building pages and forgot to add the doctype (and may not even know what that tag means).

gabrielgrant · 2022-01-02T00:09:07Z

@FredKSchott To confirm/clarify from the comments buried in the review and your notes from the RFC call -- am I understanding correctly that the decision was to auto-add doctype only in the case that an <html> element is detected?

That seems like a sensible compromise to both seamlessly avoid problems when uses forget to add doctype themselves, while still allowing for generating non-full-HTML-page content such as JSON data or @jacobrask 's use case of page fragments, an RSS feed, or dynamic SVG

Create 0000-finalize-head-body.md

55acdff

FredKSchott commented Dec 1, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

FredKSchott changed the title ~~Create 0000-finalize-head-body.md~~ Finalize <head>, <body>, <html> Dec 1, 2021

FredKSchott self-assigned this Dec 1, 2021

FredKSchott mentioned this pull request Dec 1, 2021

🐛 BUG: head contents placed in body when page has conditional head withastro/astro#2046

Closed

matthewp reviewed Dec 1, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

matthewp reviewed Dec 1, 2021

View reviewed changes

jonathantneal reviewed Dec 1, 2021

View reviewed changes

Update 0000-finalize-head-body.md

688c239

FredKSchott commented Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

FredKSchott commented Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Show resolved Hide resolved

matthewp reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

matthewp reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

FredKSchott added 3 commits December 2, 2021 08:50

Update 0000-finalize-head-body.md

4c2babb

Update 0000-finalize-head-body.md

93ab919

Update 0000-finalize-head-body.md

e9acf21

jonathantneal reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

matthewp reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

jonathantneal reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

matthewp reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Show resolved Hide resolved

matthewp reviewed Dec 2, 2021

View reviewed changes

Update active-rfcs/0000-finalize-head-body.md

cfe20bc

Co-authored-by: Jonathan Neal <jonathantneal@hotmail.com>

matthewp reviewed Dec 2, 2021

View reviewed changes

Update active-rfcs/0000-finalize-head-body.md

1089548

Co-authored-by: Jonathan Neal <jonathantneal@hotmail.com>

matthewp reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

jonathantneal reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Show resolved Hide resolved

jonathantneal reviewed Dec 2, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

jonathantneal reviewed Dec 2, 2021

View reviewed changes

FredKSchott mentioned this pull request Dec 2, 2021

Finalize v1.0 with @component tag #12

Closed

FredKSchott added 2 commits December 3, 2021 08:26

Update 0000-finalize-head-body.md

fda64a3

Update 0000-finalize-head-body.md

26037a3

FredKSchott commented Dec 3, 2021

View reviewed changes

active-rfcs/0000-finalize-head-body.md Outdated Show resolved Hide resolved

philipp-tailor mentioned this pull request Dec 5, 2021

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

Closed

FredKSchott added 3 commits December 6, 2021 14:48

Update 0000-finalize-head-body.md

d980986

Update 0000-finalize-head-body.md

889d5d6

Update 0000-finalize-head-body.md

561e49a

matthewp reviewed Dec 7, 2021

View reviewed changes

Update 0000-finalize-head-body.md

5f08fc9

FredKSchott merged commit 7451484 into main Dec 9, 2021

matthewp deleted the FredKSchott-patch-2 branch December 9, 2021 16:46

FredKSchott added the accepted label Dec 13, 2021


		## Compiler Changes

		- `src/pages` and `src/layouts` will no longer be parsed or rendered differently from other components

Conversation

FredKSchott commented Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Links

Uh oh!

Uh oh!

Uh oh!

matthewp Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

matthewp commented Dec 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathantneal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FredKSchott commented Dec 2, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FredKSchott commented Dec 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewp Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

FredKSchott Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

matthewp Dec 2, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jonathantneal left a comment

Choose a reason for hiding this comment

Uh oh!

FredKSchott commented Dec 3, 2021

Uh oh!

Uh oh!

RafidMuhymin commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthewp Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

matthewp Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewp Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

FredKSchott Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

FredKSchott commented Dec 7, 2021

Uh oh!

matthewp commented Dec 11, 2021

Uh oh!

RafidMuhymin commented Dec 15, 2021

Uh oh!

FredKSchott commented Dec 15, 2021

Uh oh!

FredKSchott commented Dec 1, 2021 •

edited

Loading

matthewp commented Dec 1, 2021 •

edited

Loading

FredKSchott commented Dec 2, 2021 •

edited

Loading

RafidMuhymin commented Dec 7, 2021 •

edited

Loading

matthewp Dec 7, 2021 •

edited

Loading

gabrielgrant commented Jan 2, 2022 •

edited

Loading