fix: conditionally import image style (#12925)

* fix: conditionally import image style

* Use wrapper component for conditional style import

* changeset

* Add tests

* Fix tests

* Update markdoc tests

* Lint

.changeset/green-buses-add.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Ensures image styles are not imported unless experimental responsive images are enabled

packages/astro/components/Image.astro

@@ -4,7 +4,6 @@ import type { UnresolvedImageTransform } from '../dist/assets/types';
 import { applyResponsiveAttributes } from '../dist/assets/utils/imageAttributes.js';
 import { AstroError, AstroErrorData } from '../dist/core/errors/index.js';
 import type { HTMLAttributes } from '../types';
-import './image.css';
 
 // The TypeScript diagnostic for JSX props uses the last member of the union to suggest props, so it would be better for
 // LocalImageProps to be last. Unfortunately, when we do this the error messages that remote images get are complete nonsense

packages/astro/components/Picture.astro

@@ -10,9 +10,8 @@ import type {
 	UnresolvedImageTransform,
 } from '../dist/types/public/index.js';
 import type { HTMLAttributes } from '../types';
-import './image.css';
 
-type Props = (LocalImageProps | RemoteImageProps) & {
+export type Props = (LocalImageProps | RemoteImageProps) & {
 	formats?: ImageOutputFormat[];
 	fallbackFormat?: ImageOutputFormat;
 	pictureAttributes?: HTMLAttributes<'picture'>;

packages/astro/components/ResponsiveImage.astro

@@ -0,0 +1,13 @@
+---
+import type { LocalImageProps, RemoteImageProps } from 'astro:assets';
+import Image from './Image.astro';
+
+type Props = LocalImageProps | RemoteImageProps;
+
+const { class: className, ...props } = Astro.props;
+
+import './image.css';
+---
+
+{/* Applying class outside of the spread prevents it from applying unnecessary astro-* classes */}
+<Image {...props} class={className} />

packages/astro/components/ResponsivePicture.astro

@@ -0,0 +1,11 @@
+---
+import { default as Picture, type Props as PictureProps } from './Picture.astro';
+
+type Props = PictureProps;
+
+const { class: className, ...props } = Astro.props;
+---
+
+{/* Applying class outside of the spread prevents it from applying unnecessary astro-* classes */}
+
+<Picture {...props} class={className} />

packages/astro/src/assets/vite-plugin-assets.ts

@@ -99,6 +99,7 @@ export default function assets({ settings }: { settings: AstroSettings }): vite.
 		referencedImages: new Set(),
 	};
 
+	const imageComponentPrefix = settings.config.experimental.responsiveImages ? 'Responsive' : '';
 	return [
 		// Expose the components and different utilities from `astro:assets`
 		{
@@ -119,8 +120,8 @@ export default function assets({ settings }: { settings: AstroSettings }): vite.
 					return /* ts */ `
 					export { getConfiguredImageService, isLocalService } from "astro/assets";
 					import { getImage as getImageInternal } from "astro/assets";
-					export { default as Image } from "astro/components/Image.astro";
-					export { default as Picture } from "astro/components/Picture.astro";
+					export { default as Image } from "astro/components/${imageComponentPrefix}Image.astro";
+					export { default as Picture } from "astro/components/${imageComponentPrefix}Picture.astro";
 					export { inferRemoteSize } from "astro/assets/utils/inferRemoteSize.js";
 
 					export const imageConfig = ${JSON.stringify({ ...settings.config.image, experimentalResponsiveImages: settings.config.experimental.responsiveImages })};

packages/astro/test/content-collections-render.test.js

@@ -26,7 +26,7 @@ describe('Content Collections - render()', () => {
 			assert.equal($('ul li').length, 3);
 
 			// Includes styles
-			assert.equal($('link[rel=stylesheet]').length, 2);
+			assert.equal($('link[rel=stylesheet]').length, 1);
 		});
 
 		it('Excludes CSS for non-rendered entries', async () => {
@@ -34,7 +34,7 @@ describe('Content Collections - render()', () => {
 			const $ = cheerio.load(html);
 
 			// Excludes styles
-			assert.equal($('link[rel=stylesheet]').length, 1);
+			assert.equal($('link[rel=stylesheet]').length, 0);
 		});
 
 		it('De-duplicates CSS used both in layout and directly in target page', async () => {
@@ -110,7 +110,7 @@ describe('Content Collections - render()', () => {
 			assert.equal($('ul li').length, 3);
 
 			// Includes styles
-			assert.equal($('link[rel=stylesheet]').length, 2);
+			assert.equal($('link[rel=stylesheet]').length, 1);
 		});
 
 		it('Exclude CSS for non-rendered entries', async () => {
@@ -121,7 +121,7 @@ describe('Content Collections - render()', () => {
 			const $ = cheerio.load(html);
 
 			// Includes styles
-			assert.equal($('link[rel=stylesheet]').length, 1);
+			assert.equal($('link[rel=stylesheet]').length, 0);
 		});
 
 		it('De-duplicates CSS used both in layout and directly in target page', async () => {

packages/astro/test/core-image.test.js

@@ -49,10 +49,11 @@ describe('astro:image', () => {
 
 		describe('basics', () => {
 			let $;
+			let body;
 			before(async () => {
 				let res = await fixture.fetch('/');
-				let html = await res.text();
-				$ = cheerio.load(html);
+				body = await res.text();
+				$ = cheerio.load(body);
 			});
 
 			it('Adds the <img> tag', () => {
@@ -61,6 +62,9 @@ describe('astro:image', () => {
 				assert.equal($img.attr('src').startsWith('/_image'), true);
 			});
 
+			it('does not inject responsive image styles when not enabled', () => {
+				assert.ok(!body.includes('[data-astro-image]'));
+			});
 			it('includes loading and decoding attributes', () => {
 				let $img = $('#local img');
 				assert.equal(!!$img.attr('loading'), true);

packages/astro/test/ssr-assets.test.js

@@ -22,7 +22,7 @@ describe('SSR Assets', () => {
 		const app = await fixture.loadTestAdapterApp();
 		/** @type {Set<string>} */
 		const assets = app.manifest.assets;
-		assert.equal(assets.size, 2);
+		assert.equal(assets.size, 1);
 		assert.equal(Array.from(assets)[0].endsWith('.css'), true);
 	});
 });

packages/integrations/markdoc/test/propagated-assets.test.js

@@ -45,12 +45,12 @@ describe('Markdoc - propagated assets', () => {
 				let styleContents;
 				if (mode === 'dev') {
 					const styles = stylesDocument.querySelectorAll('style');
-					assert.equal(styles.length, 2);
-					styleContents = styles[1].textContent;
+					assert.equal(styles.length, 1);
+					styleContents = styles[0].textContent;
 				} else {
 					const links = stylesDocument.querySelectorAll('link[rel="stylesheet"]');
-					assert.equal(links.length, 2);
-					styleContents = await fixture.readFile(links[1].href);
+					assert.equal(links.length, 1);
+					styleContents = await fixture.readFile(links[0].href);
 				}
 				assert.equal(styleContents.includes('--color-base-purple: 269, 79%;'), true);
 			});
@@ -58,10 +58,10 @@ describe('Markdoc - propagated assets', () => {
 			it('[fails] Does not bleed styles to other page', async () => {
 				if (mode === 'dev') {
 					const styles = scriptsDocument.querySelectorAll('style');
-					assert.equal(styles.length, 1);
+					assert.equal(styles.length, 0);
 				} else {
 					const links = scriptsDocument.querySelectorAll('link[rel="stylesheet"]');
-					assert.equal(links.length, 1);
+					assert.equal(links.length, 0);
 				}
 			});
 		});

packages/integrations/mdx/test/css-head-mdx.test.js

@@ -39,7 +39,7 @@ describe('Head injection w/ MDX', () => {
 			const { document } = parseHTML(html);
 
 			const links = document.querySelectorAll('head link[rel=stylesheet]');
-			assert.equal(links.length, 2);
+			assert.equal(links.length, 1);
 		});
 
 		it('injects content from a component using Content#render()', async () => {
@@ -47,7 +47,7 @@ describe('Head injection w/ MDX', () => {
 			const { document } = parseHTML(html);
 
 			const links = document.querySelectorAll('head link[rel=stylesheet]');
-			assert.equal(links.length, 2);
+			assert.equal(links.length, 1);
 
 			const scripts = document.querySelectorAll('script[type=module]');
 			assert.equal(scripts.length, 1);
@@ -79,7 +79,7 @@ describe('Head injection w/ MDX', () => {
 			const $ = cheerio.load(html);
 
 			const headLinks = $('head link[rel=stylesheet]');
-			assert.equal(headLinks.length, 2);
+			assert.equal(headLinks.length, 1);
 
 			const bodyLinks = $('body link[rel=stylesheet]');
 			assert.equal(bodyLinks.length, 0);

packages/integrations/mdx/test/fixtures/mdx-images/astro.config.ts

@@ -1,9 +1,13 @@
 import mdx from '@astrojs/mdx';
 import { testImageService } from '../../../../../astro/test/test-image-service.js';
+import { defineConfig } from 'astro/config';
 
-export default {
+export default defineConfig({
 	integrations: [mdx()],
 	image: {
 		service: testImageService(),
 	},
-}
+	experimental: {
+		responsiveImages: true,
+	}
+})

packages/integrations/mdx/test/fixtures/mdx-images/src/pages/content-collection.astro

@@ -1,9 +1,9 @@
 ---
-import { getEntry } from 'astro:content';
+import { getEntry, render } from 'astro:content';
 import MyImage from 'src/components/MyImage.astro';
 
 const entry = await getEntry('blog', 'entry');
-const { Content } = await entry.render();
+const { Content } = await render(entry)
 ---
 
 <!DOCTYPE html>

packages/integrations/mdx/test/fixtures/mdx-images/src/pages/no-image.mdx

@@ -0,0 +1 @@
+Nothing to see here.

packages/integrations/mdx/test/mdx-images.test.js

@@ -65,4 +65,18 @@ describe('MDX Page', () => {
 			});
 		}
 	});
+
+	describe('build', () => {
+		before(async () => {
+			await fixture.build();
+		});
+		it('includes responsive styles', async () => {
+			const code = await fixture.readFile('/index.html');
+			assert.ok(code.includes('[data-astro-image]'));
+		});
+		it("doesn't include styles on pages without images", async () => {
+			const code = await fixture.readFile('/no-image/index.html');
+			assert.ok(!code.includes('[data-astro-image]'));
+		});
+	});
 });