fix(inline stylesheets): did not act on propagated styles

withastro · Apr 18, 2023 · 96ffe90 · 96ffe90
1 parent 869a369
commit 96ffe90
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 46 deletions.
diff --git a/packages/astro/src/content/runtime.ts b/packages/astro/src/content/runtime.ts
@@ -6,7 +6,6 @@ import {
 	createHeadAndContent,
 	renderComponent,
 	renderScriptElement,
-	renderStyleElement,
 	renderTemplate,
 	renderUniqueStylesheet,
 	unescapeHTML,
@@ -152,13 +151,21 @@ async function render({
 				links = '',
 				scripts = '';
 			if (Array.isArray(collectedStyles)) {
-				styles = collectedStyles.map((style: any) => renderStyleElement(style)).join('');
+				styles = collectedStyles
+					.map((style: any) => {
+						return renderUniqueStylesheet(result, {
+							type: 'inline',
+							content: style,
+						});
+					})
+					.join('');
 			}
 			if (Array.isArray(collectedLinks)) {
 				links = collectedLinks
 					.map((link: any) => {
 						return renderUniqueStylesheet(result, {
-							href: prependForwardSlash(link),
+							type: 'external',
+							src: prependForwardSlash(link),
 						});
 					})
 					.join('');

diff --git a/packages/astro/src/content/vite-plugin-content-assets.ts b/packages/astro/src/content/vite-plugin-content-assets.ts
@@ -123,7 +123,8 @@ export function astroConfigBuildPlugin(
 						chunk.type === 'chunk' &&
 						(chunk.code.includes(LINKS_PLACEHOLDER) || chunk.code.includes(SCRIPTS_PLACEHOLDER))
 					) {
-						let entryCSS = new Set<string>();
+						let entryStyles = new Set<string>();
+						let entryLinks = new Set<string>();
 						let entryScripts = new Set<string>();
 
 						for (const id of Object.keys(chunk.modules)) {
@@ -137,7 +138,8 @@ export function astroConfigBuildPlugin(
 									const _entryScripts = pageData.propagatedScripts?.get(id);
 									if (_entryCss) {
 										for (const value of _entryCss) {
-											entryCSS.add(value);
+											if (value.type === 'inline') entryStyles.add(value.content);
+											if (value.type === 'external') entryLinks.add(value.src);
 										}
 									}
 									if (_entryScripts) {
@@ -150,10 +152,16 @@ export function astroConfigBuildPlugin(
 						}
 
 						let newCode = chunk.code;
-						if (entryCSS.size) {
+						if (entryStyles.size) {
+							newCode = newCode.replace(
+								JSON.stringify(STYLES_PLACEHOLDER),
+								JSON.stringify(Array.from(entryStyles))
+							);
+						}
+						if (entryLinks.size) {
 							newCode = newCode.replace(
 								JSON.stringify(LINKS_PLACEHOLDER),
-								JSON.stringify(Array.from(entryCSS).map(prependBase))
+								JSON.stringify(Array.from(entryLinks).map(prependBase))
 							);
 						}
 						if (entryScripts.size) {

diff --git a/packages/astro/src/core/build/plugins/plugin-css.ts b/packages/astro/src/core/build/plugins/plugin-css.ts
@@ -33,6 +33,7 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 
 	// stylesheet filenames are kept in here until "post", when they are rendered and ready to be inlined
 	const pagesToCss = new Map<string, Map<string, { order: number; depth: number }>>();
+	const pagesToPropagatedCss = new Map<string, Map<string, Set<string>>>();
 
 	function createNameHash(baseId: string, hashIds: string[]): string {
 		const baseName = baseId ? npath.parse(baseId).name : 'index';
@@ -189,12 +190,17 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 													const pageData = getPageDataByViteID(internals, pageViteID);
 													if (pageData) {
 														for (const css of meta.importedCss) {
+															const propagatedStyles =
+																pagesToPropagatedCss.get(pageData.moduleSpecifier) ??
+																pagesToPropagatedCss
+																	.set(pageData.moduleSpecifier, new Map())
+																	.get(pageData.moduleSpecifier)!;
+
 															const existingCss =
-																pageData.propagatedStyles.get(pageInfo.id) ?? new Set();
-															pageData.propagatedStyles.set(
-																pageInfo.id,
-																new Set([...existingCss, css])
-															);
+																propagatedStyles.get(pageInfo.id) ??
+																propagatedStyles.set(pageInfo.id, new Set()).get(pageInfo.id)!;
+
+															existingCss.add(css);
 														}
 													}
 												}
@@ -289,6 +295,24 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
 						if (orderingInfo === undefined) return;
 						pageData.styles.push({ ...orderingInfo, sheet });
 					});
+
+					pages.forEach((pageData) => {
+						const propagatedPaths = pagesToPropagatedCss.get(pageData.moduleSpecifier);
+						if (propagatedPaths === undefined) return;
+						Array.from(propagatedPaths.entries()).forEach(([pageInfoId, css]) => {
+							// return early if sheet does not need to be propagated
+							if (css.has(stylesheet.fileName) !== true) return;
+
+							// return early if the stylesheet needing propagation has already been included
+							if (pageData.styles.some((s) => s.sheet === sheet)) return;
+
+							const propagatedStyles =
+								pageData.propagatedStyles.get(pageInfoId) ??
+								pageData.propagatedStyles.set(pageInfoId, new Set()).get(pageInfoId)!;
+
+							propagatedStyles.add(sheet);
+						});
+					});
 				});
 			},
 		},

diff --git a/packages/astro/src/core/build/types.ts b/packages/astro/src/core/build/types.ts
@@ -24,7 +24,7 @@ export interface PageBuildData {
 	component: ComponentPath;
 	route: RouteData;
 	moduleSpecifier: string;
-	propagatedStyles: Map<string, Set<string>>;
+	propagatedStyles: Map<string, Set<StylesheetAsset>>;
 	propagatedScripts: Map<string, Set<string>>;
 	hoistedScript: { type: 'inline' | 'external'; value: string } | undefined;
 	styles: Array<{ depth: number; order: number; sheet: StylesheetAsset }>;

diff --git a/packages/astro/src/runtime/server/index.ts b/packages/astro/src/runtime/server/index.ts
@@ -19,7 +19,6 @@ export {
 	renderScriptElement,
 	renderSlot,
 	renderSlotToString,
-	renderStyleElement,
 	renderTemplate as render,
 	renderTemplate,
 	renderToString,

diff --git a/packages/astro/src/runtime/server/render/index.ts b/packages/astro/src/runtime/server/render/index.ts
@@ -11,6 +11,6 @@ export { renderHTMLElement } from './dom.js';
 export { maybeRenderHead, renderHead } from './head.js';
 export { renderPage } from './page.js';
 export { renderSlot, renderSlotToString, type ComponentSlots } from './slot.js';
-export { renderScriptElement, renderStyleElement, renderUniqueStylesheet } from './tags.js';
+export { renderScriptElement, renderUniqueStylesheet } from './tags.js';
 export type { RenderInstruction } from './types';
 export { addAttribute, defineScriptVars, voidElementNames } from './util.js';
diff --git a/packages/astro/src/runtime/server/render/tags.ts b/packages/astro/src/runtime/server/render/tags.ts
@@ -1,42 +1,22 @@
 import type { SSRElement, SSRResult } from '../../../@types/astro';
+import type { StylesheetAsset } from '../../../core/app/types';
 import { renderElement } from './util.js';
 
-const stylesheetRel = 'stylesheet';
-
-export function renderStyleElement(children: string) {
-	return renderElement('style', {
-		props: {},
-		children,
-	});
-}
-
 export function renderScriptElement({ props, children }: SSRElement) {
 	return renderElement('script', {
 		props,
 		children,
 	});
 }
 
-export function renderStylesheet({ href }: { href: string }) {
-	return renderElement(
-		'link',
-		{
-			props: {
-				rel: stylesheetRel,
-				href,
-			},
-			children: '',
-		},
-		false
-	);
-}
-
-export function renderUniqueStylesheet(result: SSRResult, link: { href: string }) {
-	for (const existingLink of result.links) {
-		if (existingLink.props.rel === stylesheetRel && existingLink.props.href === link.href) {
-			return '';
-		}
+export function renderUniqueStylesheet(result: SSRResult, sheet: StylesheetAsset) {
+	if (sheet.type === 'external') {
+		if (Array.from(result.styles).some((s) => s.props.href === sheet.src)) return '';
+		return renderElement('link', { props: { rel: 'stylesheet', href: sheet.src }, children: '' });
 	}
 
-	return renderStylesheet(link);
+	if (sheet.type === 'inline') {
+		if (Array.from(result.styles).some((s) => s.children === sheet.content)) return '';
+		return renderElement('style', { props: { type: 'text/css' }, children: sheet.content });
+	}
 }
diff --git a/packages/astro/test/units/dev/head-injection.test.js b/packages/astro/test/units/dev/head-injection.test.js
@@ -35,7 +35,8 @@ describe('head injection', () => {
 							factory(result, props, slots) {
 								return createHeadAndContent(
 									unescapeHTML(renderUniqueStylesheet(result, {
-										href: '/some/fake/styles.css'
+										type: 'external',
+										src: '/some/fake/styles.css'
 									})),
 									renderTemplate\`$\{renderComponent(result, 'Other', Other, props, slots)}\`
 								);
@@ -113,7 +114,8 @@ describe('head injection', () => {
 							factory(result, props, slots) {
 								return createHeadAndContent(
 									unescapeHTML(renderUniqueStylesheet(result, {
-										href: '/some/fake/styles.css'
+										type: 'external',
+										src: '/some/fake/styles.css'
 									})),
 									renderTemplate\`$\{renderComponent(result, 'Other', Other, props, slots)}\`
 								);