From b55f76c1cafb7918f7087c6df03dd1d59eeaa065 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Mon, 15 Aug 2022 13:10:28 -0400 Subject: [PATCH] Fix double injecting of head content in md pages (#4334) * Fix double injecting of head content * Refactor * Changeset * Break into a separate util fn * fix oops * remove unused code --- .changeset/hip-bobcats-divide.md | 5 ++++ .../astro/src/runtime/server/render/astro.ts | 4 +++ .../src/runtime/server/render/component.ts | 4 +-- .../astro/src/runtime/server/render/page.ts | 28 +++++++++++++++---- .../astro/src/vite-plugin-markdown/index.ts | 1 + .../fixtures/head-injection-md/package.json | 8 ++++++ .../src/components/Layout.astro | 18 ++++++++++++ .../head-injection-md/src/pages/index.md | 7 +++++ packages/astro/test/head-injection-md.test.js | 27 ++++++++++++++++++ pnpm-lock.yaml | 6 ++++ 10 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 .changeset/hip-bobcats-divide.md create mode 100644 packages/astro/test/fixtures/head-injection-md/package.json create mode 100644 packages/astro/test/fixtures/head-injection-md/src/components/Layout.astro create mode 100644 packages/astro/test/fixtures/head-injection-md/src/pages/index.md create mode 100644 packages/astro/test/head-injection-md.test.js diff --git a/.changeset/hip-bobcats-divide.md b/.changeset/hip-bobcats-divide.md new file mode 100644 index 000000000..838e1bc7f --- /dev/null +++ b/.changeset/hip-bobcats-divide.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix double injecting of head content in md pages diff --git a/packages/astro/src/runtime/server/render/astro.ts b/packages/astro/src/runtime/server/render/astro.ts index b7e47a68c..f6ec92acb 100644 --- a/packages/astro/src/runtime/server/render/astro.ts +++ b/packages/astro/src/runtime/server/render/astro.ts @@ -56,6 +56,10 @@ export function isAstroComponent(obj: any): obj is AstroComponent { ); } +export function isAstroComponentFactory(obj: any): obj is AstroComponentFactory { + return obj == null ? false : !!obj.isAstroComponentFactory; +} + export async function* renderAstroComponent( component: InstanceType ): AsyncIterable { diff --git a/packages/astro/src/runtime/server/render/component.ts b/packages/astro/src/runtime/server/render/component.ts index 18dc5a00d..7bf5f7e56 100644 --- a/packages/astro/src/runtime/server/render/component.ts +++ b/packages/astro/src/runtime/server/render/component.ts @@ -6,7 +6,7 @@ import { extractDirectives, generateHydrateScript } from '../hydration.js'; import { serializeProps } from '../serialize.js'; import { shorthash } from '../shorthash.js'; import { renderSlot } from './any.js'; -import { renderAstroComponent, renderTemplate, renderToIterable } from './astro.js'; +import { isAstroComponentFactory, renderAstroComponent, renderTemplate, renderToIterable } from './astro.js'; import { Fragment, Renderer } from './common.js'; import { componentIsHTMLElement, renderHTMLElement } from './dom.js'; import { formatList, internalSpreadAttributes, renderElement, voidElementNames } from './util.js'; @@ -37,7 +37,7 @@ function getComponentType(Component: unknown): ComponentType { if (Component && typeof Component === 'object' && (Component as any)['astro:html']) { return 'html'; } - if (Component && (Component as any).isAstroComponentFactory) { + if (isAstroComponentFactory(Component)) { return 'astro-factory'; } return 'unknown'; diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index 9e2c0fd2b..c2a3921c7 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -2,21 +2,34 @@ import type { SSRResult } from '../../../@types/astro'; import type { AstroComponentFactory } from './index'; import { createResponse } from '../response.js'; -import { isAstroComponent, renderAstroComponent } from './astro.js'; +import { isAstroComponent, isAstroComponentFactory, renderAstroComponent } from './astro.js'; import { stringifyChunk } from './common.js'; import { renderComponent } from './component.js'; import { maybeRenderHead } from './head.js'; const encoder = new TextEncoder(); +const needsHeadRenderingSymbol = Symbol.for('astro.needsHeadRendering'); + +type NonAstroPageComponent = { + name: string; + [needsHeadRenderingSymbol]: boolean; +}; + +function nonAstroPageNeedsHeadInjection(pageComponent: NonAstroPageComponent): boolean { + return ( + (needsHeadRenderingSymbol in pageComponent) && + !!pageComponent[needsHeadRenderingSymbol] + ); +} export async function renderPage( result: SSRResult, - componentFactory: AstroComponentFactory, + componentFactory: AstroComponentFactory | NonAstroPageComponent, props: any, children: any, streaming: boolean ): Promise { - if (!componentFactory.isAstroComponentFactory) { + if (!isAstroComponentFactory(componentFactory)) { const pageProps: Record = { ...(props ?? {}), 'server:root': true }; const output = await renderComponent( result, @@ -29,8 +42,13 @@ export async function renderPage( if (!/`; - for await (let chunk of maybeRenderHead(result)) { - html += chunk; + // This symbol currently exists for md components, but is something that could + // be added for any page-level component that's not an Astro component. + // to signal that head rendering is needed. + if(nonAstroPageNeedsHeadInjection(componentFactory)) { + for await (let chunk of maybeRenderHead(result)) { + html += chunk; + } } html += rest; } diff --git a/packages/astro/src/vite-plugin-markdown/index.ts b/packages/astro/src/vite-plugin-markdown/index.ts index 53b584f4a..acaefed24 100644 --- a/packages/astro/src/vite-plugin-markdown/index.ts +++ b/packages/astro/src/vite-plugin-markdown/index.ts @@ -117,6 +117,7 @@ export default function markdown({ config, logging }: AstroPluginOptions): Plugi : `contentFragment` }; } + Content[Symbol.for('astro.needsHeadRendering')] = ${layout ? 'false' : 'true'}; export default Content; `); diff --git a/packages/astro/test/fixtures/head-injection-md/package.json b/packages/astro/test/fixtures/head-injection-md/package.json new file mode 100644 index 000000000..d2f2c6778 --- /dev/null +++ b/packages/astro/test/fixtures/head-injection-md/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/head-injection-md", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/head-injection-md/src/components/Layout.astro b/packages/astro/test/fixtures/head-injection-md/src/components/Layout.astro new file mode 100644 index 000000000..225a16a12 --- /dev/null +++ b/packages/astro/test/fixtures/head-injection-md/src/components/Layout.astro @@ -0,0 +1,18 @@ +--- +const title = 'My Title'; +--- + + + + + + + + + + + diff --git a/packages/astro/test/fixtures/head-injection-md/src/pages/index.md b/packages/astro/test/fixtures/head-injection-md/src/pages/index.md new file mode 100644 index 000000000..f32c4c3d6 --- /dev/null +++ b/packages/astro/test/fixtures/head-injection-md/src/pages/index.md @@ -0,0 +1,7 @@ +--- +layout: ../components/Layout.astro +--- + +# Heading + +And content here. diff --git a/packages/astro/test/head-injection-md.test.js b/packages/astro/test/head-injection-md.test.js new file mode 100644 index 000000000..1acd5e83b --- /dev/null +++ b/packages/astro/test/head-injection-md.test.js @@ -0,0 +1,27 @@ +import { expect } from 'chai'; +import * as cheerio from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +describe('Head injection with markdown', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/head-injection-md/', + }); + }); + + describe('build', () => { + before(async () => { + await fixture.build(); + }); + + it('only injects head content once', async () => { + const html = await fixture.readFile(`/index.html`); + const $ = cheerio.load(html); + + expect($('link[rel=stylesheet]')).to.have.a.lengthOf(1); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 72a051de7..788c6ac14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1531,6 +1531,12 @@ importers: dependencies: astro: link:../../.. + packages/astro/test/fixtures/head-injection-md: + specifiers: + astro: workspace:* + dependencies: + astro: link:../../.. + packages/astro/test/fixtures/hmr-css: specifiers: astro: workspace:*