Fix Astro HMR from a CSS dependency (#8609)

* Fix Astro HMR from a CSS dependency * Improve css test * Create wise-donuts-tickle.md
withastro · Sep 21, 2023 · 5a988ea · 5a988ea
1 parent e8c997d
commit 5a988ea
Show file tree

Hide file tree

Showing 12 changed files with 54 additions and 35 deletions.
diff --git a/.changeset/wise-donuts-tickle.md b/.changeset/wise-donuts-tickle.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fix Astro HMR from a CSS dependency
diff --git a/packages/astro/e2e/astro-component.test.js b/packages/astro/e2e/astro-component.test.js
@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import { getColor, testFactory } from './test-utils.js';
+import { testFactory } from './test-utils.js';
 
 const test = testFactory({ root: './fixtures/astro-component/' });
 
@@ -99,14 +99,14 @@ test.describe('Astro component HMR', () => {
 	test('update linked dep Astro style', async ({ page, astro }) => {
 		await page.goto(astro.resolveUrl('/'));
 		let h1 = page.locator('#astro-linked-lib');
-		expect(await getColor(h1)).toBe('rgb(255, 0, 0)');
+		await expect(h1).toHaveCSS('color', 'rgb(255, 0, 0)');
 		await Promise.all([
 			page.waitForLoadState('networkidle'),
 			await astro.editFile('../_deps/astro-linked-lib/Component.astro', (content) =>
 				content.replace('color: red', 'color: green')
 			),
 		]);
 		h1 = page.locator('#astro-linked-lib');
-		expect(await getColor(h1)).toBe('rgb(0, 128, 0)');
+		await expect(h1).toHaveCSS('color', 'rgb(0, 128, 0)');
 	});
 });
diff --git a/packages/astro/e2e/css.test.js b/packages/astro/e2e/css.test.js
@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import { getColor, testFactory } from './test-utils.js';
+import { testFactory } from './test-utils.js';
 
 const test = testFactory({
 	root: './fixtures/css/',
@@ -20,13 +20,13 @@ test.describe('CSS HMR', () => {
 		await page.goto(astro.resolveUrl('/'));
 
 		const h = page.locator('h1');
-		expect(await getColor(h)).toBe('rgb(255, 0, 0)');
+		await expect(h).toHaveCSS('color', 'rgb(255, 0, 0)');
 
 		await astro.editFile('./src/styles/main.css', (original) =>
 			original.replace('--h1-color: red;', '--h1-color: green;')
 		);
 
-		expect(await getColor(h)).toBe('rgb(0, 128, 0)');
+		await expect(h).toHaveCSS('color', 'rgb(0, 128, 0)');
 	});
 
 	test('removes Astro-injected CSS once Vite-injected CSS loads', async ({ page, astro }) => {

diff --git a/...tures/invalidate-script-deps/package.json → packages/astro/e2e/fixtures/hmr/package.json b/...tures/invalidate-script-deps/package.json → packages/astro/e2e/fixtures/hmr/package.json
@@ -1,8 +1,9 @@
 {
-  "name": "@e2e/invalidate-script-deps",
+  "name": "@e2e/hmr",
   "version": "0.0.0",
   "private": true,
   "devDependencies": {
-    "astro": "workspace:*"
+    "astro": "workspace:*",
+    "sass": "^1.66.1"
   }
 }
diff --git a/packages/astro/e2e/fixtures/hmr/src/pages/css-dep.astro b/packages/astro/e2e/fixtures/hmr/src/pages/css-dep.astro
@@ -0,0 +1,14 @@
+<html>
+	<head>
+		<title>Test</title>
+	</head>
+	<body>
+		<h1 class="css-dep">This is blue</h1>
+		<style lang="scss">
+			@use "../styles/vars.scss" as *;
+			.css-dep {
+				color: $color;
+			}
+		</style>
+	</body>
+</html>
diff --git a/...alidate-script-deps/src/pages/index.astro → ...e/fixtures/hmr/src/pages/script-dep.astro b/...alidate-script-deps/src/pages/index.astro → ...e/fixtures/hmr/src/pages/script-dep.astro
diff --git a/...lidate-script-deps/src/scripts/heading.js → ...o/e2e/fixtures/hmr/src/scripts/heading.js b/...lidate-script-deps/src/scripts/heading.js → ...o/e2e/fixtures/hmr/src/scripts/heading.js
diff --git a/packages/astro/e2e/fixtures/hmr/src/styles/vars.scss b/packages/astro/e2e/fixtures/hmr/src/styles/vars.scss
@@ -0,0 +1 @@
+$color: blue;
diff --git a/.../astro/e2e/invalidate-script-deps.test.js → packages/astro/e2e/hmr.test.js b/.../astro/e2e/invalidate-script-deps.test.js → packages/astro/e2e/hmr.test.js
@@ -2,7 +2,7 @@ import { expect } from '@playwright/test';
 import { testFactory } from './test-utils.js';
 
 const test = testFactory({
-	root: './fixtures/invalidate-script-deps/',
+	root: './fixtures/hmr/',
 });
 
 let devServer;
@@ -17,7 +17,7 @@ test.afterAll(async () => {
 
 test.describe('Scripts with dependencies', () => {
 	test('refresh with HMR', async ({ page, astro }) => {
-		await page.goto(astro.resolveUrl('/'));
+		await page.goto(astro.resolveUrl('/script-dep'));
 
 		const h = page.locator('h1');
 		await expect(h, 'original text set').toHaveText('before');
@@ -29,3 +29,16 @@ test.describe('Scripts with dependencies', () => {
 		await expect(h, 'text changed').toHaveText('after');
 	});
 });
+
+test.describe('Styles with dependencies', () => {
+	test('refresh with HMR', async ({ page, astro }) => {
+		await page.goto(astro.resolveUrl('/css-dep'));
+
+		const h = page.locator('h1');
+		await expect(h).toHaveCSS('color', 'rgb(0, 0, 255)');
+
+		await astro.editFile('./src/styles/vars.scss', (original) => original.replace('blue', 'red'));
+
+		await expect(h).toHaveCSS('color', 'rgb(255, 0, 0)');
+	});
+});
diff --git a/packages/astro/e2e/test-utils.js b/packages/astro/e2e/test-utils.js
@@ -71,13 +71,6 @@ export async function getErrorOverlayContent(page) {
 	return { message, hint, absoluteFileLocation, fileLocation };
 }
 
-/**
- * @returns {Promise<string>}
- */
-export async function getColor(el) {
-	return await el.evaluate((e) => getComputedStyle(e).color);
-}
-
 /**
  * Wait for `astro-island` that contains the `el` to hydrate
  * @param {import('@playwright/test').Page} page

diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts
@@ -90,7 +90,7 @@ export async function handleHotUpdate(
 
 	// Bugfix: sometimes style URLs get normalized and end with `lang.css=`
 	// These will cause full reloads, so filter them out here
-	const mods = ctx.modules.filter((m) => !m.url.endsWith('='));
+	const mods = [...filtered].filter((m) => !m.url.endsWith('='));
 	const file = ctx.file.replace(config.root.pathname, '/');
 
 	// If only styles are changed, remove the component file from the update list
@@ -109,17 +109,6 @@ export async function handleHotUpdate(
 		}
 	}
 
-	// If this is a module that is imported from a <script>, invalidate the Astro
-	// component so that it is cached by the time the script gets transformed.
-	for (const mod of filtered) {
-		if (mod.id && isAstroScript(mod.id) && mod.file) {
-			const astroMod = ctx.server.moduleGraph.getModuleById(mod.file);
-			if (astroMod) {
-				mods.unshift(astroMod);
-			}
-		}
-	}
-
 	// TODO: Svelte files should be marked as `isSelfAccepting` but they don't appear to be
 	const isSelfAccepting = mods.every((m) => m.isSelfAccepting || m.url.endsWith('.svelte'));
 	if (isSelfAccepting) {

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