Error recovery test and more (#3388)

* Add test to verify errors are recovered from * Fix nested style components not be added in dev on initial load * Adds a changeset
withastro · May 17, 2022 · 4d00473 · 4d00473
1 parent 5a81ef4
commit 4d00473
Show file tree

Hide file tree

Showing 14 changed files with 275 additions and 14 deletions.
diff --git a/.changeset/lucky-meals-ring.md b/.changeset/lucky-meals-ring.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes nested style bug causing initial styles to be off
diff --git a/packages/astro/e2e/fixtures/nested-styles/package.json b/packages/astro/e2e/fixtures/nested-styles/package.json
@@ -0,0 +1,8 @@
+{
+  "name": "@test/nested-style-bug-e22e",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}
diff --git a/packages/astro/e2e/fixtures/nested-styles/src/components/Card.astro b/packages/astro/e2e/fixtures/nested-styles/src/components/Card.astro
@@ -0,0 +1,75 @@
+---
+export interface Props {
+    title: string,
+    body: string,
+    href: string,
+}
+const {href, title, body} = Astro.props;
+---
+<li class="link-card">
+    <a href={href}>
+        <h2>
+            {title}
+            <span>&rarr;</span>
+        </h2>
+        <p>
+            {body}
+            <slot name="icon" />
+        </p>
+    </a>
+</li>
+<style>
+
+	:root {
+		--link-gradient: linear-gradient(45deg, #4F39FA, #DA62C4 30%, var(--color-border) 60%);
+	}
+
+	.link-card {
+		list-style: none;
+		display: flex;
+		padding: 0.15rem;
+		background-image: var(--link-gradient);
+		background-size: 400%;
+		border-radius: 0.5rem;
+		background-position: 100%;
+		transition: background-position 0.6s cubic-bezier(0.22, 1, 0.36, 1);
+	}
+
+	.link-card > a {
+		width: 100%;
+		text-decoration: none;
+		line-height: 1.4;
+		padding: 1em 1.3em;
+		border-radius: 0.35rem;
+		color: var(--text-color);
+		background-color: white;
+		opacity: 0.8;
+	}
+
+	h2 {
+		margin: 0;
+		transition: color 0.6s cubic-bezier(0.22, 1, 0.36, 1);
+	}
+
+    p {
+        margin-top: 0.75rem;
+        margin-bottom: 0;
+    }
+
+	h2 span {
+		display: inline-block;
+		transition: transform 0.3s cubic-bezier(0.22, 1, 0.36, 1);
+	}
+
+	.link-card:is(:hover, :focus-within) {
+		background-position: 0;
+	}
+
+	.link-card:is(:hover, :focus-within) h2 {
+		color: #4F39FA;
+	}
+
+	.link-card:is(:hover, :focus-within) h2 span {
+		transform: translateX(2px);
+	}
+</style>
diff --git a/packages/astro/e2e/fixtures/nested-styles/src/components/partials/Header/index.astro b/packages/astro/e2e/fixtures/nested-styles/src/components/partials/Header/index.astro
@@ -0,0 +1,24 @@
+<style>
+	header {
+		display: flex;
+		position: relative;
+		align-items: center;
+		justify-content: space-between;
+		padding: 20px;
+		background: darkblue;
+		color: white;
+	}
+
+	.title {
+		display: block;
+		width: 180px;
+	}
+</style>
+
+
+<header>
+	<a class="title" href="/">
+		<span>My Website</span>
+	</a>
+</header>
+
diff --git a/packages/astro/e2e/fixtures/nested-styles/src/components/partials/Layout/index.astro b/packages/astro/e2e/fixtures/nested-styles/src/components/partials/Layout/index.astro
@@ -0,0 +1,18 @@
+---
+import Header from '../Header/index.astro'
+
+import '../../../styles/global.css'
+---
+<html>
+	<head>
+		<meta charset="utf-8" />
+		<meta http-equiv="x-ua-compatible" content="ie=edge" />
+		<meta name="viewport" content="width=device-width,initial-scale=1,shrink-to-fit=no" />
+	</head>
+	<body>
+		<Header />
+		<main>
+			<slot />
+		</main>
+	</body>
+</html>
diff --git a/packages/astro/e2e/fixtures/nested-styles/src/pages/index.astro b/packages/astro/e2e/fixtures/nested-styles/src/pages/index.astro
@@ -0,0 +1,13 @@
+---
+import Layout from '../components/partials/Layout/index.astro';
+
+
+const content = {
+	title: 'My Website'
+}
+---
+
+<Layout content={content}>
+	<h1 class="title">My Website</h1>
+	<p>This is my website</p>
+</Layout>
diff --git a/packages/astro/e2e/fixtures/nested-styles/src/styles/global.css b/packages/astro/e2e/fixtures/nested-styles/src/styles/global.css
@@ -0,0 +1,19 @@
+body {
+  font-family: sans-serif;
+  font-size: 16px;
+  font-weight: 300;
+  line-height: 1.12;
+}
+
+a {
+  color: inherit;
+  text-decoration: underline;
+}
+
+h1,
+h2,
+h3,
+h4,
+h5 {
+  font-weight: 700;
+}
diff --git a/packages/astro/e2e/nested-styles.test.js b/packages/astro/e2e/nested-styles.test.js
@@ -0,0 +1,32 @@
+import { test as base, expect } from '@playwright/test';
+import { loadFixture } from './test-utils.js';
+
+const test = base.extend({
+	astro: async ({}, use) => {
+		const fixture = await loadFixture({ root: './fixtures/nested-styles/' });
+		await use(fixture);
+	},
+});
+
+let devServer;
+
+test.beforeAll(async ({ astro }) => {
+	devServer = await astro.startDevServer();
+});
+
+test.afterAll(async ({ astro }) => {
+	await devServer.stop();
+});
+
+test('Loading styles that are nested', async ({ page, astro }) => {
+	await page.goto(astro.resolveUrl('/'));
+
+	await test.step('header', async () => {
+		const header = page.locator('header');
+
+		await expect(header, 'should have background color').toHaveCSS(
+			'background-color',
+			'rgb(0, 0, 139)' // darkblue
+		);
+	});
+});
diff --git a/packages/astro/src/core/dev/index.ts b/packages/astro/src/core/dev/index.ts
@@ -23,6 +23,7 @@ export interface DevOptions {
 
 export interface DevServer {
 	address: AddressInfo;
+	watcher: vite.FSWatcher;
 	stop(): Promise<void>;
 }
 
@@ -69,6 +70,9 @@ export default async function dev(config: AstroConfig, options: DevOptions): Pro
 
 	return {
 		address: devServerAddressInfo,
+		get watcher() {
+			return viteServer.watcher;
+		},
 		stop: async () => {
 			await viteServer.close();
 			await runHookServerDone({ config });

diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts
@@ -316,7 +316,6 @@ async function handleRequest(
 			return await writeSSRResult(result, res, statusCode);
 		}
 	} catch (_err) {
-		debugger;
 		const err = fixViteErrorMessage(createSafeError(_err), viteServer);
 		error(logging, null, msg.formatErrorMessage(err));
 		handle500Response(viteServer, origin, req, res, err);

diff --git a/packages/astro/src/vite-plugin-astro/index.ts b/packages/astro/src/vite-plugin-astro/index.ts
@@ -198,16 +198,9 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
 						i++;
 					}
 
-					// We only need to define deps if there are any
-					if (deps.size > 1) {
-						SUFFIX += `\nif(import.meta.hot) import.meta.hot.accept(["${id}", "${Array.from(
-							deps
-						).join('","')}"], (...mods) => mods);`;
-					} else {
-						SUFFIX += `\nif (import.meta.hot) {
-							import.meta.hot.accept(mod => mod);
-						}`;
-					}
+					SUFFIX += `\nif (import.meta.hot) {
+						import.meta.hot.accept(mod => mod);
+					}`;
 				}
 				// Add handling to inject scripts into each page JS bundle, if needed.
 				if (isPage) {

diff --git a/packages/astro/test/errors.test.js b/packages/astro/test/errors.test.js
@@ -1,5 +1,6 @@
 import { isWindows, loadFixture } from './test-utils.js';
 import { expect } from 'chai';
+import * as cheerio from 'cheerio';
 
 describe('Error display', () => {
 	if (isWindows) return;
@@ -30,4 +31,34 @@ describe('Error display', () => {
 			expect(res.status).to.equal(500, `Successfully responded with 500 Error for invalid file`);
 		});
 	});
+
+	describe('Framework components', () => {
+		let devServer;
+
+		before(async () => {
+			devServer = await fixture.startDevServer();
+		});
+
+		after(async () => {
+			await devServer.stop();
+		});
+
+		it('Errors recover when fixed', async () => {
+			let html = await fixture.fetch('/svelte-syntax-error').then((res) => res.text());
+
+			// 1. Verify an error message is being shown.
+			let $ = cheerio.load(html);
+			expect($('.statusMessage').text()).to.equal('Internal Error');
+
+			// 2. Edit the file, fixing the error
+			let changeOccured = fixture.onNextChange();
+			await fixture.editFile('./src/components/SvelteSyntaxError.svelte', `<h1>No mismatch</h1>`);
+			await changeOccured;
+
+			// 3. Verify that the file is fixed.
+			html = await fixture.fetch('/svelte-syntax-error').then((res) => res.text());
+			$ = cheerio.load(html);
+			expect($('h1').text()).to.equal('No mismatch');
+		});
+	});
 });
diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js
@@ -97,12 +97,31 @@ export async function loadFixture(inlineConfig) {
 	const resolveUrl = (url) =>
 		`http://${'127.0.0.1'}:${config.server.port}${url.replace(/^\/?/, '/')}`;
 
+	// A map of files that have been editted.
+	let fileEdits = new Map();
+
+	const resetAllFiles = () => {
+		for(const [, reset] of fileEdits) {
+			reset();
+		}
+		fileEdits.clear();
+	};
+
+	// After each test, reset each of the edits to their original contents.
+	if(typeof afterEach === 'function') {
+		afterEach(resetAllFiles);
+	}
+	// Also do it on process exit, just in case.
+	process.on('exit', resetAllFiles);
+
+	let devServer;
+
 	return {
 		build: (opts = {}) => build(config, { mode: 'development', logging, telemetry, ...opts }),
 		startDevServer: async (opts = {}) => {
-			const devResult = await dev(config, { logging, telemetry, ...opts });
-			config.server.port = devResult.address.port; // update port
-			return devResult;
+			devServer = await dev(config, { logging, telemetry, ...opts });
+			config.server.port = devServer.address.port; // update port
+			return devServer;
 		},
 		config,
 		resolveUrl,
@@ -120,6 +139,21 @@ export async function loadFixture(inlineConfig) {
 			const { createApp } = await import(url);
 			return createApp();
 		},
+		editFile: async (filePath, newContents) => {
+			const fileUrl = new URL(filePath.replace(/^\//, ''), config.root);
+			const contents = await fs.promises.readFile(fileUrl, 'utf-8');
+			const reset = () => fs.writeFileSync(fileUrl, contents);
+			// Only save this reset if not already in the map, in case multiple edits happen
+			// to the same file.
+			if(!fileEdits.has(fileUrl.toString())) {
+				fileEdits.set(fileUrl.toString(), reset);
+			}
+			await fs.promises.writeFile(fileUrl, newContents);
+			return reset;
+		},
+		onNextChange: () => devServer ?
+			new Promise(resolve => devServer.watcher.once('change', resolve)) :
+			Promise.reject(new Error('No dev server running')),
 	};
 }
 

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