Handle layouts and head injection (#1007)

* Revert "Revert "Fixes duplicate head content (#995)" (#1001)"

This reverts commit b1959f0fed.

* Handle markdown layouts

* Adding a changeset

.changeset/kind-crabs-marry.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a previous revert, makes sure head content is injected into the right place

packages/astro/src/compiler/codegen/index.ts

@@ -662,7 +662,7 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
                   buffers[curr] += `h(__astro_slot_content, { name: ${attributes.slot} },`;
                   paren++;
                 }
-                buffers[curr] += `h("${name}", ${generateAttributes(attributes)}`;
+                buffers[curr] += `h("${name}", ${generateAttributes(attributes)},`;
                 paren++;
                 return;
               }
@@ -692,7 +692,7 @@ async function compileHtml(enterNode: TemplateNode, state: CodegenState, compile
                   buffers[curr] += `h(__astro_slot_content, { name: ${attributes.slot} },`;
                   paren++;
                 }
-                buffers[curr] += `h(${componentName}, ${generateAttributes(attributes)}`;
+                buffers[curr] += `h(${componentName}, ${generateAttributes(attributes)},`;
                 paren++;
                 return;
               } else if (!componentInfo && !isCustomElementTag(componentName)) {

packages/astro/src/compiler/index.ts

@@ -150,8 +150,8 @@ async function __render(props, ...children) {
       value: props,
       enumerable: true
     },
-    css: {
-      value: (props[__astroInternal] && props[__astroInternal].css) || [],
+    pageCSS: {
+      value: (props[__astroContext] && props[__astroContext].pageCSS) || [],
       enumerable: true
     },
     isPage: {
@@ -179,18 +179,21 @@ export async function __renderPage({request, children, props, css}) {
     __render,
   };
 
-  Object.defineProperty(props, __astroContext, {
-    value: {
-      request
-    },
-    writable: false,
-    enumerable: false
-  });
+  const isLayout = (__astroContext in props);
+  if(!isLayout) {
+    Object.defineProperty(props, __astroContext, {
+      value: {
+        pageCSS: css,
+        request
+      },
+      writable: false,
+      enumerable: false
+    });
+  }
 
   Object.defineProperty(props, __astroInternal, {
     value: {
-      css,
-      isPage: true
+      isPage: !isLayout
     },
     writable: false,
     enumerable: false
@@ -202,7 +205,7 @@ export async function __renderPage({request, children, props, css}) {
   if (currentChild.layout) {
     return currentChild.layout({
       request,
-      props: {content: currentChild.content},
+      props: {content: currentChild.content, [__astroContext]: props[__astroContext]},
       children: [childBodyResult],
     });
   }

packages/astro/src/compiler/transform/head.ts

@@ -73,7 +73,7 @@ export default function (opts: TransformOptions): Transformer {
             start: 0,
             end: 0,
             type: 'Expression',
-            codeChunks: ['Astro.css.map(css => (', '))'],
+            codeChunks: ['Astro.pageCSS.map(css => (', '))'],
             children: [
               {
                 type: 'Element',
@@ -162,22 +162,15 @@ export default function (opts: TransformOptions): Transformer {
         );
       }
 
-      const conditionalNode = {
-        start: 0,
-        end: 0,
-        type: 'Expression',
-        codeChunks: ['Astro.isPage ? (', ') : null'],
-        children: [
-          {
-            start: 0,
-            end: 0,
-            type: 'Fragment',
-            children,
-          },
-        ],
-      };
-
-      eoh.append(conditionalNode);
+      if (eoh.foundHeadOrHtmlElement || eoh.foundHeadAndBodyContent) {
+        const topLevelFragment = {
+          start: 0,
+          end: 0,
+          type: 'Fragment',
+          children,
+        };
+        eoh.append(topLevelFragment);
+      }
     },
   };
 }

packages/astro/src/compiler/transform/util/end-of-head.ts

@@ -1,21 +1,44 @@
 import type { TemplateNode } from '@astrojs/parser';
 
-const validHeadElements = new Set(['!doctype', 'title', 'meta', 'link', 'style', 'script', 'noscript', 'base']);
+const beforeHeadElements = new Set(['!doctype', 'html']);
+const validHeadElements = new Set(['title', 'meta', 'link', 'style', 'script', 'noscript', 'base']);
 
 export class EndOfHead {
+  private html: TemplateNode | null = null;
   private head: TemplateNode | null = null;
   private firstNonHead: TemplateNode | null = null;
   private parent: TemplateNode | null = null;
   private stack: TemplateNode[] = [];
 
+  public foundHeadElements = false;
+  public foundBodyElements = false;
   public append: (...node: TemplateNode[]) => void = () => void 0;
 
   get found(): boolean {
     return !!(this.head || this.firstNonHead);
   }
 
+  get foundHeadContent(): boolean {
+    return !!this.head || this.foundHeadElements;
+  }
+
+  get foundHeadAndBodyContent(): boolean {
+    return this.foundHeadContent && this.foundBodyElements;
+  }
+
+  get foundHeadOrHtmlElement(): boolean {
+    return !!(this.html || this.head);
+  }
+
   enter(node: TemplateNode) {
+    const name = node.name ? node.name.toLowerCase() : null;
+
     if (this.found) {
+      if (!validHeadElements.has(name)) {
+        if (node.type === 'Element') {
+          this.foundBodyElements = true;
+        }
+      }
       return;
     }
 
@@ -26,8 +49,6 @@ export class EndOfHead {
       return;
     }
 
-    const name = node.name.toLowerCase();
-
     if (name === 'head') {
       this.head = node;
       this.parent = this.stack[this.stack.length - 2];
@@ -35,11 +56,24 @@ export class EndOfHead {
       return;
     }
 
+    // Skip !doctype and html elements
+    if (beforeHeadElements.has(name)) {
+      if (name === 'html') {
+        this.html = node;
+      }
+      return;
+    }
+
     if (!validHeadElements.has(name)) {
+      if (node.type === 'Element') {
+        this.foundBodyElements = true;
+      }
       this.firstNonHead = node;
       this.parent = this.stack[this.stack.length - 2];
       this.append = this.prependToFirstNonHead;
       return;
+    } else {
+      this.foundHeadElements = true;
     }
   }
 

packages/astro/test/astro-doctype.test.js

@@ -1,38 +1,13 @@
-import { fileURLToPath } from 'url';
 import { suite } from 'uvu';
 import * as assert from 'uvu/assert';
-import { loadConfig } from '#astro/config';
-import { createRuntime } from '#astro/runtime';
+import { doc } from './test-utils.js';
+import { setup } from './helpers.js';
 
 const DType = suite('doctype');
 
-let runtime, setupError;
+setup(DType, './fixtures/astro-doctype');
 
-DType.before(async () => {
-  try {
-    const astroConfig = await loadConfig(fileURLToPath(new URL('./fixtures/astro-doctype', import.meta.url)));
-
-    const logging = {
-      level: 'error',
-      dest: process.stderr,
-    };
-
-    runtime = await createRuntime(astroConfig, { logging });
-  } catch (err) {
-    console.error(err);
-    setupError = err;
-  }
-});
-
-DType.after(async () => {
-  (await runtime) && runtime.shutdown();
-});
-
-DType('No errors creating a runtime', () => {
-  assert.equal(setupError, undefined);
-});
-
-DType('Automatically prepends the standards mode doctype', async () => {
+DType('Automatically prepends the standards mode doctype', async ({ runtime }) => {
   const result = await runtime.load('/prepend');
   assert.ok(!result.error, `build error: ${result.error}`);
 
@@ -40,7 +15,7 @@ DType('Automatically prepends the standards mode doctype', async () => {
   assert.ok(html.startsWith('<!doctype html>'), 'Doctype always included');
 });
 
-DType('No attributes added when doctype is provided by user', async () => {
+DType('No attributes added when doctype is provided by user', async ({ runtime }) => {
   const result = await runtime.load('/provided');
   assert.ok(!result.error, `build error: ${result.error}`);
 
@@ -48,7 +23,7 @@ DType('No attributes added when doctype is provided by user', async () => {
   assert.ok(html.startsWith('<!doctype html>'), 'Doctype always included');
 });
 
-DType.skip('Preserves user provided doctype', async () => {
+DType.skip('Preserves user provided doctype', async ({ runtime }) => {
   const result = await runtime.load('/preserve');
   assert.ok(!result.error, `build error: ${result.error}`);
 
@@ -56,7 +31,7 @@ DType.skip('Preserves user provided doctype', async () => {
   assert.ok(html.startsWith('<!doctype HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">'), 'Doctype included was preserved');
 });
 
-DType('User provided doctype is case insensitive', async () => {
+DType('User provided doctype is case insensitive', async ({ runtime }) => {
   const result = await runtime.load('/capital');
   assert.ok(!result.error, `build error: ${result.error}`);
 
@@ -65,4 +40,34 @@ DType('User provided doctype is case insensitive', async () => {
   assert.not.ok(html.includes('</!DOCTYPE>'), 'There should not be a closing tag');
 });
 
+DType('Doctype can be provided in a layout', async ({ runtime }) => {
+  const result = await runtime.load('/in-layout');
+  assert.ok(!result.error, `build error: ${result.error}`);
+
+  const html = result.contents.toString('utf-8');
+  assert.ok(html.startsWith('<!doctype html>'), 'doctype is at the front');
+
+  const $ = doc(html);
+  assert.equal($('head link').length, 1, 'A link inside of the head');
+});
+
+DType('Doctype is added in a layout without one', async ({ runtime }) => {
+  const result = await runtime.load('/in-layout-no-doctype');
+  assert.ok(!result.error, `build error: ${result.error}`);
+
+  const html = result.contents.toString('utf-8');
+  assert.ok(html.startsWith('<!doctype html>'), 'doctype is at the front');
+});
+
+DType('Doctype is added in a layout used with markdown pages', async ({ runtime }) => {
+  const result = await runtime.load('/in-layout-article');
+  assert.ok(!result.error, `build error: ${result.error}`);
+
+  const html = result.contents.toString('utf-8');
+  assert.ok(html.startsWith('<!doctype html>'), 'doctype is at the front');
+
+  const $ = doc(html);
+  assert.equal($('head link').length, 1, 'A link inside of the head');
+});
+
 DType.run();

packages/astro/test/fixtures/astro-doctype/src/components/Meta.astro

@@ -0,0 +1,5 @@
+---
+import SubMeta from './SubMeta.astro';
+---
+<meta name="author" content="Astro Fan">
+<SubMeta />

packages/astro/test/fixtures/astro-doctype/src/components/SubMeta.astro

@@ -0,0 +1 @@
+<meta name="keywords" content="JavaScript,Astro">

packages/astro/test/fixtures/astro-doctype/src/layouts/WithDoctype.astro

@@ -0,0 +1,14 @@
+---
+import '../styles/global.css';
+import Meta from '../components/Meta.astro';
+---
+<!doctype html>
+<html lang="en">
+  <head>
+    <title>My App</title>
+    <Meta />
+  </head>
+  <body>
+    <slot />
+  </body>
+</html>

packages/astro/test/fixtures/astro-doctype/src/layouts/WithoutDoctype.astro

@@ -0,0 +1,11 @@
+---
+import '../styles/global.css'
+---
+<html lang="en">
+  <head>
+    <title>My App</title>
+  </head>
+  <body>
+
+  </body>
+</html>

packages/astro/test/fixtures/astro-doctype/src/pages/in-layout-article.md

@@ -0,0 +1,7 @@
+---
+layout: ../layouts/WithDoctype.astro
+---
+
+# Some Title
+
+Some content

packages/astro/test/fixtures/astro-doctype/src/pages/in-layout-no-doctype.astro

@@ -0,0 +1,4 @@
+---
+import WithoutDoctype from '../layouts/WithoutDoctype.astro';
+---
+<WithoutDoctype />

packages/astro/test/fixtures/astro-doctype/src/pages/in-layout.astro

@@ -0,0 +1,4 @@
+---
+import WithDoctype from '../layouts/WithDoctype.astro';
+---
+<WithDoctype />

packages/astro/test/fixtures/astro-doctype/src/styles/global.css

@@ -0,0 +1,3 @@
+body {
+  background: green;
+}

packages/astro/test/fixtures/no-head-el/src/pages/index.astro

@@ -2,6 +2,7 @@
 import Something from '../components/Something.jsx';
 import Child from '../components/Child.astro';
 ---
+
 <title>My page</title>
 <style>
   .h1 {

packages/astro/test/fixtures/no-head-el/src/pages/no-elements.astro

@@ -1,6 +0,0 @@
----
-import Something from '../components/Something.jsx';
-import Child from '../components/Child.astro';
----
-<Something client:load />
-<Child />

packages/astro/test/no-head-el.test.js

@@ -25,14 +25,4 @@ NoHeadEl('Places style and scripts before the first non-head element', async ({
   assert.equal($('script[src="/_snowpack/hmr-client.js"]').length, 1, 'Only the hmr client for the page');
 });
 
-NoHeadEl('Injects HMR script even when there are no elements on the page', async ({ runtime }) => {
-  const result = await runtime.load('/no-elements');
-  assert.ok(!result.error, `build error: ${result.error}`);
-
-  const html = result.contents;
-  const $ = doc(html);
-
-  assert.equal($('script[src="/_snowpack/hmr-client.js"]').length, 1, 'Only the hmr client for the page');
-});
-
 NoHeadEl.run();