Remove post-rendering head injection (#3679)

* Remove post-rendering head injection * Adds a changeset * Use a layout component for vue
withastro · Jun 23, 2022 · fa7ed3f · fa7ed3f
1 parent 446f8c4
commit fa7ed3f
Show file tree

Hide file tree

Showing 22 changed files with 74 additions and 44 deletions.
diff --git a/.changeset/tasty-hornets-return.md b/.changeset/tasty-hornets-return.md
@@ -0,0 +1,11 @@
+---
+'astro': patch
+---
+
+Moves head injection to happen during rendering
+
+This change makes it so that head injection; to insert component stylesheets, hoisted scripts, for example, to happen during rendering than as a post-rendering step.
+
+This is to enable streaming. This change will only be noticeable if you are rendering your `<head>` element inside of a framework component. If that is the case then the head items will be injected before the first non-head element in an Astro file instead.
+
+In the future we may offer a `<Astro.Head>` component as a way to control where these scripts/styles are inserted.
diff --git a/packages/astro/e2e/fixtures/preact-component/src/components/Layout.astro b/packages/astro/e2e/fixtures/preact-component/src/components/Layout.astro
@@ -0,0 +1,4 @@
+<html>
+	<head><title>Preact component</title></head>
+	<body><slot></slot></body>
+</html>
diff --git a/packages/astro/e2e/fixtures/preact-component/src/pages/markdown.md b/packages/astro/e2e/fixtures/preact-component/src/pages/markdown.md
@@ -1,4 +1,5 @@
 ---
+layout: ../components/Layout.astro
 setup: |
   import Counter from '../components/Counter.jsx';
   import PreactComponent from '../components/JSXComponent.jsx';

diff --git a/packages/astro/e2e/fixtures/react-component/src/components/Layout.astro b/packages/astro/e2e/fixtures/react-component/src/components/Layout.astro
@@ -0,0 +1,4 @@
+<html>
+	<head><title>React component</title></head>
+	<body><slot></slot></body>
+</html>
diff --git a/packages/astro/e2e/fixtures/react-component/src/pages/markdown.md b/packages/astro/e2e/fixtures/react-component/src/pages/markdown.md
@@ -1,4 +1,5 @@
 ---
+layout: ../components/Layout.astro
 setup: |
   import Counter from '../components/Counter.jsx';
   import ReactComponent from '../components/JSXComponent.jsx';

diff --git a/packages/astro/e2e/fixtures/solid-component/src/components/Layout.astro b/packages/astro/e2e/fixtures/solid-component/src/components/Layout.astro
@@ -0,0 +1,4 @@
+<html>
+	<head><title>Solid component</title></head>
+	<body><slot></slot></body>
+</html>
diff --git a/packages/astro/e2e/fixtures/solid-component/src/pages/markdown.md b/packages/astro/e2e/fixtures/solid-component/src/pages/markdown.md
@@ -1,4 +1,5 @@
 ---
+layout: ../components/Layout.astro
 setup: |
   import Counter from '../components/Counter.jsx';
   import SolidComponent from '../components/SolidComponent.jsx';

diff --git a/packages/astro/e2e/fixtures/svelte-component/src/components/Layout.astro b/packages/astro/e2e/fixtures/svelte-component/src/components/Layout.astro
@@ -0,0 +1,4 @@
+<html>
+	<head><title>Solid component</title></head>
+	<body><slot></slot></body>
+</html>
diff --git a/packages/astro/e2e/fixtures/svelte-component/src/pages/markdown.md b/packages/astro/e2e/fixtures/svelte-component/src/pages/markdown.md
@@ -1,4 +1,5 @@
 ---
+layout: ../components/Layout.astro
 setup: |
   import Counter from '../components/Counter.svelte';
   import SvelteComponent from '../components/SvelteComponent.svelte';

diff --git a/packages/astro/e2e/fixtures/vue-component/src/components/Layout.astro b/packages/astro/e2e/fixtures/vue-component/src/components/Layout.astro
@@ -0,0 +1,4 @@
+<html>
+	<head><title>Vue component</title></head>
+	<body><slot></slot></body>
+</html>
diff --git a/packages/astro/e2e/fixtures/vue-component/src/pages/markdown.md b/packages/astro/e2e/fixtures/vue-component/src/pages/markdown.md
@@ -1,4 +1,5 @@
 ---
+layout: ../components/Layout.astro
 setup: |
   import Counter from '../components/Counter.vue';
   import VueComponent from '../components/VueComponent.vue';

diff --git a/packages/astro/package.json b/packages/astro/package.json
@@ -78,7 +78,7 @@
     "test:e2e:match": "playwright test -g"
   },
   "dependencies": {
-    "@astrojs/compiler": "^0.16.1",
+    "@astrojs/compiler": "^0.17.0",
     "@astrojs/language-server": "^0.13.4",
     "@astrojs/markdown-remark": "^0.11.3",
     "@astrojs/prism": "0.4.1",

diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -1004,7 +1004,6 @@ export interface SSRElement {
 export interface SSRMetadata {
 	renderers: SSRLoadedRenderer[];
 	pathname: string;
-	needsHydrationStyles: boolean;
 }
 
 export interface SSRResult {

diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts
@@ -161,12 +161,6 @@ export async function render(
 	}
 
 	let html = page.html;
-	// handle final head injection if it hasn't happened already
-	if (html.indexOf('<!--astro:head:injected-->') == -1) {
-		html = (await renderHead(result)) + html;
-	}
-	// cleanup internal state flags
-	html = html.replace('<!--astro:head:injected-->', '');
 
 	// inject <!doctype html> if missing (TODO: is a more robust check needed for comments, etc.?)
 	if (!/<!doctype html/i.test(html)) {

diff --git a/packages/astro/src/core/render/result.ts b/packages/astro/src/core/render/result.ts
@@ -221,7 +221,6 @@ ${extra}`
 		},
 		resolve,
 		_metadata: {
-			needsHydrationStyles: false,
 			renderers,
 			pathname,
 		},

diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
@@ -344,7 +344,6 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 		{ renderer: renderer!, result, astroId, props },
 		metadata as Required<AstroComponentMetadata>
 	);
-	result._metadata.needsHydrationStyles = true;
 
 	// Render template if not all astro fragments are provided.
 	let unrenderedSlots: string[] = [];
@@ -590,16 +589,6 @@ Update your code to remove this warning.`);
 	return handler.call(mod, proxy, request);
 }
 
-async function replaceHeadInjection(result: SSRResult, html: string): Promise<string> {
-	let template = html;
-	// <!--astro:head--> injected by compiler
-	// Must be handled at the end of the rendering process
-	if (template.indexOf('<!--astro:head-->') > -1) {
-		template = template.replace('<!--astro:head-->', await renderHead(result));
-	}
-	return template;
-}
-
 // Calls a component and renders it into a string of HTML
 export async function renderToString(
 	result: SSRResult,
@@ -627,8 +616,7 @@ export async function renderPage(
 		const response = await componentFactory(result, props, children);
 
 		if (isAstroComponent(response)) {
-			let template = await renderAstroComponent(response);
-			const html = await replaceHeadInjection(result, template);
+			let html = await renderAstroComponent(response);
 			return {
 				type: 'html',
 				html,
@@ -660,37 +648,36 @@ const uniqueElements = (item: any, index: number, all: any[]) => {
 	);
 };
 
-// Renders a page to completion by first calling the factory callback, waiting for its result, and then appending
-// styles and scripts into the head.
+const alreadyHeadRenderedResults = new WeakSet<SSRResult>();
 export async function renderHead(result: SSRResult): Promise<string> {
+	alreadyHeadRenderedResults.add(result);
 	const styles = Array.from(result.styles)
 		.filter(uniqueElements)
 		.map((style) => renderElement('style', style));
-	let needsHydrationStyles = result._metadata.needsHydrationStyles;
 	const scripts = Array.from(result.scripts)
 		.filter(uniqueElements)
 		.map((script, i) => {
-			if ('data-astro-component-hydration' in script.props) {
-				needsHydrationStyles = true;
-			}
 			return renderElement('script', script);
 		});
-	if (needsHydrationStyles) {
-		styles.push(
-			renderElement('style', {
-				props: {},
-				children: 'astro-island, astro-slot { display: contents; }',
-			})
-		);
-	}
 	const links = Array.from(result.links)
 		.filter(uniqueElements)
 		.map((link) => renderElement('link', link, false));
 	return markHTMLString(
-		links.join('\n') + styles.join('\n') + scripts.join('\n') + '\n' + '<!--astro:head:injected-->'
+		links.join('\n') + styles.join('\n') + scripts.join('\n')
 	);
 }
 
+// This function is called by Astro components that do not contain a <head> component
+// This accomodates the fact that using a <head> is optional in Astro, so this
+// is called before a component's first non-head HTML element. If the head was 
+// already injected it is a noop. 
+export function maybeRenderHead(result: SSRResult): string | Promise<string> {
+	if(alreadyHeadRenderedResults.has(result)) {
+		return '';
+	}
+	return renderHead(result);
+}
+
 export async function renderAstroComponent(component: InstanceType<typeof AstroComponent>) {
 	let template = [];
 

diff --git a/packages/astro/src/runtime/server/scripts.ts b/packages/astro/src/runtime/server/scripts.ts
@@ -59,7 +59,7 @@ export function getPrescripts(type: PrescriptType, directive: string): string {
 	// deps to be loaded immediately.
 	switch (type) {
 		case 'both':
-			return `<script>${getDirectiveScriptText(directive) + islandScript}</script>`;
+			return `<style>astro-island,astro-slot{display:contents}</style><script>${getDirectiveScriptText(directive) + islandScript}</script>`;
 		case 'directive':
 			return `<script>${getDirectiveScriptText(directive)}</script>`;
 	}

diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js
@@ -65,7 +65,7 @@ describe('CSS', function () {
 
 			it('Using hydrated components adds astro-island styles', async () => {
 				const inline = $('style').html();
-				expect(inline).to.include('display: contents');
+				expect(inline).to.include('display:contents');
 			});
 
 			it('<style lang="sass">', async () => {

diff --git a/packages/astro/test/astro-partial-html.test.js b/packages/astro/test/astro-partial-html.test.js
@@ -40,4 +40,10 @@ describe('Partial HTML', async () => {
 		const allInjectedStyles = $('style[data-astro-injected]').text().replace(/\s*/g, '');
 		expect(allInjectedStyles).to.match(/h1{color:red;}/);
 	});
+
+	it('pages with a head, injection happens inside', async () => {
+		const html = await fixture.fetch('/with-head').then((res) => res.text());
+		const $ = cheerio.load(html);
+		expect($('style')).to.have.lengthOf(1);
+	});
 });
diff --git a/packages/astro/test/fixtures/astro-partial-html/src/pages/with-head.astro b/packages/astro/test/fixtures/astro-partial-html/src/pages/with-head.astro
@@ -0,0 +1,9 @@
+<html>
+	<head>
+		<title>testing</title>
+		<style>body { color: blue; }</style>
+	</head>
+	<body>
+		<h1>testing</h1>
+	</body>
+</html>
diff --git a/packages/webapi/mod.d.ts b/packages/webapi/mod.d.ts
@@ -1,5 +1,5 @@
 export { pathToPosix } from './lib/utils';
-export { AbortController, AbortSignal, alert, atob, Blob, btoa, ByteLengthQueuingStrategy, cancelAnimationFrame, cancelIdleCallback, CanvasRenderingContext2D, CharacterData, clearTimeout, Comment, CountQueuingStrategy, CSSStyleSheet, CustomElementRegistry, CustomEvent, Document, DocumentFragment, DOMException, Element, Event, EventTarget, fetch, File, FormData, Headers, HTMLBodyElement, HTMLCanvasElement, HTMLDivElement, HTMLDocument, HTMLElement, HTMLHeadElement, HTMLHtmlElement, HTMLImageElement, HTMLSpanElement, HTMLStyleElement, HTMLTemplateElement, HTMLUnknownElement, Image, ImageData, IntersectionObserver, MediaQueryList, MutationObserver, Node, NodeFilter, NodeIterator, OffscreenCanvas, ReadableByteStreamController, ReadableStream, ReadableStreamBYOBReader, ReadableStreamBYOBRequest, ReadableStreamDefaultController, ReadableStreamDefaultReader, Request, requestAnimationFrame, requestIdleCallback, ResizeObserver, Response, setTimeout, ShadowRoot, structuredClone, StyleSheet, Text, TransformStream, TreeWalker, URLPattern, Window, WritableStream, WritableStreamDefaultController, WritableStreamDefaultWriter } from './mod.js';
+export { AbortController, AbortSignal, alert, atob, Blob, btoa, ByteLengthQueuingStrategy, cancelAnimationFrame, cancelIdleCallback, CanvasRenderingContext2D, CharacterData, clearTimeout, Comment, CountQueuingStrategy, CSSStyleSheet, CustomElementRegistry, CustomEvent, Document, DocumentFragment, DOMException, Element, Event, EventTarget, fetch, File, FormData, Headers, HTMLBodyElement, HTMLCanvasElement, HTMLDivElement, HTMLDocument, HTMLElement, HTMLHeadElement, HTMLHtmlElement, HTMLImageElement, HTMLSpanElement, HTMLStyleElement, HTMLTemplateElement, HTMLUnknownElement, Image, ImageData, IntersectionObserver, MediaQueryList, MutationObserver, Node, NodeFilter, NodeIterator, OffscreenCanvas, ReadableByteStreamController, ReadableStream, ReadableStreamBYOBReader, ReadableStreamBYOBRequest, ReadableStreamDefaultController, ReadableStreamDefaultReader, Request, requestAnimationFrame, requestIdleCallback, ResizeObserver, Response, setTimeout, ShadowRoot, structuredClone, StyleSheet, Text, TransformStream, TreeWalker, URLPattern, Window, WritableStream, WritableStreamDefaultController, WritableStreamDefaultWriter, } from './mod.js';
 export declare const polyfill: {
     (target: any, options?: PolyfillOptions): any;
     internals(target: any, name: string): any;

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml