From 4d00473dbd20e673b5c9857d500dee072bb20f99 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 17 May 2022 11:43:49 -0400 Subject: [PATCH] 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 --- .changeset/lucky-meals-ring.md | 5 ++ .../e2e/fixtures/nested-styles/package.json | 8 ++ .../nested-styles/src/components/Card.astro | 75 +++++++++++++++++++ .../components/partials/Header/index.astro | 24 ++++++ .../components/partials/Layout/index.astro | 18 +++++ .../nested-styles/src/pages/index.astro | 13 ++++ .../nested-styles/src/styles/global.css | 19 +++++ packages/astro/e2e/nested-styles.test.js | 32 ++++++++ packages/astro/src/core/dev/index.ts | 4 + .../src/vite-plugin-astro-server/index.ts | 1 - packages/astro/src/vite-plugin-astro/index.ts | 13 +--- packages/astro/test/errors.test.js | 31 ++++++++ packages/astro/test/test-utils.js | 40 +++++++++- pnpm-lock.yaml | 6 ++ 14 files changed, 275 insertions(+), 14 deletions(-) create mode 100644 .changeset/lucky-meals-ring.md create mode 100644 packages/astro/e2e/fixtures/nested-styles/package.json create mode 100644 packages/astro/e2e/fixtures/nested-styles/src/components/Card.astro create mode 100644 packages/astro/e2e/fixtures/nested-styles/src/components/partials/Header/index.astro create mode 100644 packages/astro/e2e/fixtures/nested-styles/src/components/partials/Layout/index.astro create mode 100644 packages/astro/e2e/fixtures/nested-styles/src/pages/index.astro create mode 100644 packages/astro/e2e/fixtures/nested-styles/src/styles/global.css create mode 100644 packages/astro/e2e/nested-styles.test.js diff --git a/.changeset/lucky-meals-ring.md b/.changeset/lucky-meals-ring.md new file mode 100644 index 000000000..e1f703e9a --- /dev/null +++ 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 new file mode 100644 index 000000000..fd48a3ea4 --- /dev/null +++ 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 new file mode 100644 index 000000000..7b911b395 --- /dev/null +++ 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; +--- +
  • + +

    + {title} + → +

    +

    + {body} + +

    +     +
    • + \ No newline at end of file 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 new file mode 100644 index 000000000..5e1e0b75b --- /dev/null +++ b/packages/astro/e2e/fixtures/nested-styles/src/components/partials/Header/index.astro @@ -0,0 +1,24 @@ + + + +
    + + My Website + +
    + 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 new file mode 100644 index 000000000..231ce8645 --- /dev/null +++ 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' +--- + + + + + + + +
    +
    + +
    + + diff --git a/packages/astro/e2e/fixtures/nested-styles/src/pages/index.astro b/packages/astro/e2e/fixtures/nested-styles/src/pages/index.astro new file mode 100644 index 000000000..b09a7ac63 --- /dev/null +++ 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' +} +--- + + +

    My Website

    +

    This is my website

    +     \ No newline at end of file diff --git a/packages/astro/e2e/fixtures/nested-styles/src/styles/global.css b/packages/astro/e2e/fixtures/nested-styles/src/styles/global.css new file mode 100644 index 000000000..47bfab093 --- /dev/null +++ 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 new file mode 100644 index 000000000..7c233d4cb --- /dev/null +++ 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 index 76051c623..eb9591b4e 100644 --- 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; } @@ -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 index bcbbfd35f..a71d6d788 100644 --- 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 index f37bbd652..b13b85494 100644 --- 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 index 85baee002..3f1b2b2e8 100644 --- 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', `

    No mismatch

    `); + 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 index 5d56965ad..e4268b4d3 100644 --- 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 index 59461efe9..e45a5113c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -661,6 +661,12 @@ importers: chai-as-promised: 7.1.1_chai@4.3.6 mocha: 9.2.2 + packages/astro/e2e/fixtures/nested-styles: + specifiers: + astro: workspace:* + dependencies: + astro: link:../../.. + packages/astro/e2e/fixtures/tailwindcss: specifiers: '@astrojs/tailwind': workspace:*