Skip to content

Commit

Permalink
Migrate more tests to the static build (#2693)
Browse files Browse the repository at this point in the history 
* fix: disable HMR during build (#2684)

* Migrate more tests to the static build

* Only prepend links in non-legacy mode

* Add the 0-css tests

* Convert all CSS tests to the static build

* Migrate Astro global tests

* Remove .only

* Fix static build tests

* Migrate a few more

* More tests

* Move the lit test back to legacy

* Increase the test timeout

Co-authored-by: Nate Moore <[email protected]>
  • Loading branch information
@matthewp @natemoo-re
matthewp and natemoo-re authored Mar 2, 2022
1 parent c8cbe4b commit 8f08197
Show file tree
Hide file tree
Showing 47 changed files with 150 additions and 419 deletions.
5 changes: 5 additions & 0 deletions .changeset/wet-knives-raise.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix issue where HMR could be triggered during `astro build`
4 changes: 2 additions & 2 deletions packages/astro/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
"dev": "astro-scripts dev \"src/**/*.ts\"",
"postbuild": "astro-scripts copy \"src/**/*.astro\"",
"benchmark": "node test/benchmark/dev.bench.js && node test/benchmark/build.bench.js",
"test": "mocha --parallel --timeout 15000 --ignore **/lit-element.test.js && mocha **/lit-element.test.js",
"test:match": "mocha --timeout 15000 -g"
"test": "mocha --parallel --timeout 20000 --ignore **/lit-element.test.js && mocha --timeout 20000 **/lit-element.test.js",
"test:match": "mocha --timeout 20000 -g"
},
"dependencies": {
"@astrojs/compiler": "^0.12.0-next.5",
Expand Down
18 changes: 10 additions & 8 deletions packages/astro/src/core/build/static-build.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ export interface StaticBuildOptions {
const MAX_CONCURRENT_RENDERS = 10;

function addPageName(pathname: string, opts: StaticBuildOptions): void {
const pathrepl = opts.astroConfig.buildOptions.pageUrlFormat === 'directory' ? '/index.html' : pathname === '/' ? 'index.html' : '.html';
opts.pageNames.push(pathname.replace(/\/?$/, pathrepl).replace(/^\//, ''));
opts.pageNames.push(pathname.replace(/\/?$/, '/').replace(/^\//, ''));
}

// Gives back a facadeId that is relative to the root.
Expand Down Expand Up @@ -362,7 +361,7 @@ async function generatePath(pathname: string, opts: StaticBuildOptions, gopts: G
debug('build', `Generating: ${pathname}`);

const site = astroConfig.buildOptions.site;
const links = createLinkStylesheetElementSet(linkIds, site);
const links = createLinkStylesheetElementSet(linkIds.reverse(), site);
const scripts = createModuleScriptElementWithSrcSet(hoistedId ? [hoistedId] : [], site);

try {
Expand Down Expand Up @@ -456,8 +455,7 @@ async function generateManifest(result: RollupOutput, opts: StaticBuildOptions,
}

function getOutRoot(astroConfig: AstroConfig): URL {
const rootPathname = appendForwardSlash(astroConfig.buildOptions.site ? new URL(astroConfig.buildOptions.site).pathname : '/');
return new URL('.' + rootPathname, astroConfig.dist);
return new URL('./', astroConfig.dist);
}

function getServerRoot(astroConfig: AstroConfig): URL {
Expand All @@ -483,8 +481,10 @@ function getOutFolder(astroConfig: AstroConfig, pathname: string, routeType: Rou
switch (astroConfig.buildOptions.pageUrlFormat) {
case 'directory':
return new URL('.' + appendForwardSlash(pathname), outRoot);
case 'file':
case 'file': {
return new URL('.' + appendForwardSlash(npath.dirname(pathname)), outRoot);
}

}
}
}
Expand All @@ -497,8 +497,10 @@ function getOutFile(astroConfig: AstroConfig, outFolder: URL, pathname: string,
switch (astroConfig.buildOptions.pageUrlFormat) {
case 'directory':
return new URL('./index.html', outFolder);
case 'file':
return new URL('./' + npath.basename(pathname) + '.html', outFolder);
case 'file': {
const baseName = npath.basename(pathname);
return new URL('./' + (baseName || 'index') + '.html', outFolder);
}
}
}
}
Expand Down
74 changes: 50 additions & 24 deletions packages/astro/src/core/render/dev/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type * as vite from 'vite';
import type { AstroConfig, ComponentInstance, Renderer, RouteData, RuntimeMode } from '../../../@types/astro';
import type { AstroConfig, ComponentInstance, Renderer, RouteData, RuntimeMode, SSRElement } from '../../../@types/astro';
import { LogOptions } from '../../logger.js';
import { fileURLToPath } from 'url';
import { getStylesForURL } from './css.js';
Expand Down Expand Up @@ -48,14 +48,15 @@ export async function preload({ astroConfig, filePath, viteServer }: SSROptions)
/** use Vite to SSR */
export async function render(renderers: Renderer[], mod: ComponentInstance, ssrOpts: SSROptions): Promise<string> {
const { astroConfig, filePath, logging, mode, origin, pathname, route, routeCache, viteServer } = ssrOpts;
const legacy = astroConfig.buildOptions.legacyBuild;

// Add hoisted script tags
const scripts = createModuleScriptElementWithSrcSet(
!astroConfig.buildOptions.legacyBuild && mod.hasOwnProperty('$$metadata') ? Array.from(mod.$$metadata.hoistedScriptPaths()) : []
!legacy && mod.hasOwnProperty('$$metadata') ? Array.from(mod.$$metadata.hoistedScriptPaths()) : []
);

// Inject HMR scripts
if (mod.hasOwnProperty('$$metadata') && mode === 'development' && !astroConfig.buildOptions.legacyBuild) {
if (mod.hasOwnProperty('$$metadata') && mode === 'development' && !legacy) {
scripts.add({
props: { type: 'module', src: '/@vite/client' },
children: '',
Expand All @@ -66,9 +67,31 @@ export async function render(renderers: Renderer[], mod: ComponentInstance, ssrO
});
}

// Pass framework CSS in as link tags to be appended to the page.
let links = new Set<SSRElement>();
if(!legacy) {
[...getStylesForURL(filePath, viteServer)].forEach((href) => {
if (mode === 'development' && svelteStylesRE.test(href)) {
scripts.add({
props: { type: 'module', src: href },
children: '',
});
} else {
links.add({
props: {
rel: 'stylesheet',
href,
'data-astro-injected': true,
},
children: '',
});
}
});
}

let content = await coreRender({
legacyBuild: astroConfig.buildOptions.legacyBuild,
links: new Set(),
links,
logging,
markdownRender: astroConfig.markdownOptions.render,
mod,
Expand Down Expand Up @@ -101,7 +124,7 @@ export async function render(renderers: Renderer[], mod: ComponentInstance, ssrO
const tags: vite.HtmlTagDescriptor[] = [];

// dev only: inject Astro HMR client
if (mode === 'development' && astroConfig.buildOptions.legacyBuild) {
if (mode === 'development' && legacy) {
tags.push({
tag: 'script',
attrs: { type: 'module' },
Expand All @@ -113,25 +136,28 @@ export async function render(renderers: Renderer[], mod: ComponentInstance, ssrO
}

// inject CSS
[...getStylesForURL(filePath, viteServer)].forEach((href) => {
if (mode === 'development' && svelteStylesRE.test(href)) {
tags.push({
tag: 'script',
attrs: { type: 'module', src: href },
injectTo: 'head',
});
} else {
tags.push({
tag: 'link',
attrs: {
rel: 'stylesheet',
href,
'data-astro-injected': true,
},
injectTo: 'head',
});
}
});
if(legacy) {
[...getStylesForURL(filePath, viteServer)].forEach((href) => {
if (mode === 'development' && svelteStylesRE.test(href)) {
tags.push({
tag: 'script',
attrs: { type: 'module', src: href },
injectTo: 'head',
});
} else {
tags.push({
tag: 'link',
attrs: {
rel: 'stylesheet',
href,
'data-astro-injected': true,
},
injectTo: 'head',
});
}
});
}


// add injected tags
content = injectTags(content, tags);
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/render/sitemap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const STATUS_CODE_PAGE_REGEXP = /\/[0-9]{3}\/?$/;
/** Construct sitemap.xml given a set of URLs */
export function generateSitemap(pages: string[]): string {
// TODO: find way to respect <link rel="canonical"> URLs here

// TODO: find way to exclude pages from sitemap

// copy just in case original copy is needed
Expand Down
18 changes: 10 additions & 8 deletions packages/astro/src/runtime/server/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,14 @@ export function createAstro(filePathname: string, _site: string, projectRootStr:
};
}

const toAttributeString = (value: any) => String(value).replace(/&/g, '&#38;').replace(/"/g, '&#34;');
const toAttributeString = (value: any, shouldEscape = true) => shouldEscape ?
String(value).replace(/&/g, '&#38;').replace(/"/g, '&#34;') :
value;

const STATIC_DIRECTIVES = new Set(['set:html', 'set:text']);

// A helper used to turn expressions into attribute key/value
export function addAttribute(value: any, key: string) {
export function addAttribute(value: any, key: string, shouldEscape = true) {
if (value == null) {
return '';
}
Expand Down Expand Up @@ -372,15 +374,15 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) {
return unescapeHTML(` ${key}`);
} else {
return unescapeHTML(` ${key}="${toAttributeString(value)}"`);
return unescapeHTML(` ${key}="${toAttributeString(value, shouldEscape)}"`);
}
}

// Adds support for `<Component {...value} />
export function spreadAttributes(values: Record<any, any>) {
export function spreadAttributes(values: Record<any, any>, shouldEscape = true) {
let output = '';
for (const [key, value] of Object.entries(values)) {
output += addAttribute(value, key);
output += addAttribute(value, key, shouldEscape);
}
return unescapeHTML(output);
}
Expand Down Expand Up @@ -463,7 +465,7 @@ export async function renderPage(result: SSRResult, Component: AstroComponentFac

const links = Array.from(result.links)
.filter(uniqueElements)
.map((link) => renderElement('link', link));
.map((link) => renderElement('link', link, false));

// inject styles & scripts at end of <head>
let headPos = template.indexOf('</head>');
Expand Down Expand Up @@ -513,7 +515,7 @@ function getHTMLElementName(constructor: typeof HTMLElement) {
return assignedName;
}

function renderElement(name: string, { props: _props, children = '' }: SSRElement) {
function renderElement(name: string, { props: _props, children = '' }: SSRElement, shouldEscape = true) {
// Do not print `hoist`, `lang`, `global`
const { lang: _, 'data-astro-id': astroId, 'define:vars': defineVars, ...props } = _props;
if (defineVars) {
Expand All @@ -530,5 +532,5 @@ function renderElement(name: string, { props: _props, children = '' }: SSRElemen
children = defineScriptVars(defineVars) + '\n' + children;
}
}
return `<${name}${spreadAttributes(props)}>${children}</${name}>`;
return `<${name}${spreadAttributes(props, shouldEscape)}>${children}</${name}>`;
}
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ export default function astro({ config, logging }: AstroPluginOptions): vite.Plu
}
},
async handleHotUpdate(context) {
if (context.server.config.isProduction) return;
return handleHotUpdate(context, config, logging);
},
};
Expand Down
25 changes: 13 additions & 12 deletions packages/astro/test/0-css.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@ describe('CSS', function () {
fixture = await loadFixture({
projectRoot: './fixtures/0-css/',
renderers: ['@astrojs/renderer-react', '@astrojs/renderer-svelte', '@astrojs/renderer-vue'],
buildOptions: { legacyBuild: true } // TODO make this test work without legacyBuild
vite: {
build: {
assetsInlineLimit: 0
}
}
});
});

Expand All @@ -32,7 +36,7 @@ describe('CSS', function () {
// get bundled CSS (will be hashed, hence DOM query)
const html = await fixture.readFile('/index.html');
$ = cheerio.load(html);
const bundledCSSHREF = $('link[rel=stylesheet][href^=./assets/]').attr('href');
const bundledCSSHREF = $('link[rel=stylesheet][href^=/assets/]').attr('href');
bundledCSS = await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/'));
});

Expand All @@ -48,7 +52,8 @@ describe('CSS', function () {
expect(el2.attr('class')).to.equal(`visible ${scopedClass}`);

// 2. check CSS
expect(bundledCSS).to.include(`.blue.${scopedClass}{color:#b0e0e6}.color\\:blue.${scopedClass}{color:#b0e0e6}.visible.${scopedClass}{display:block}`);
const expected = `.blue.${scopedClass}{color:#b0e0e6}.color\\\\:blue.${scopedClass}{color:#b0e0e6}.visible.${scopedClass}{display:block}`;
expect(bundledCSS).to.include(expected);
});

it('No <style> skips scoping', async () => {
Expand All @@ -61,7 +66,8 @@ describe('CSS', function () {
});

it('Using hydrated components adds astro-root styles', async () => {
expect(bundledCSS).to.include('display:contents');
const inline = $('style').html();
expect(inline).to.include('display: contents');
});

it('<style lang="sass">', async () => {
Expand Down Expand Up @@ -218,7 +224,7 @@ describe('CSS', function () {
it('<style>', async () => {
const el = $('#svelte-css');
const classes = el.attr('class').split(' ');
const scopedClass = classes.find((name) => /^s-[A-Za-z0-9-]+/.test(name));
const scopedClass = classes.find((name) => name !== 'svelte-css' && /^svelte-[A-Za-z0-9-]+/.test(name));

// 1. check HTML
expect(el.attr('class')).to.include('svelte-css');
Expand All @@ -230,7 +236,7 @@ describe('CSS', function () {
it('<style lang="sass">', async () => {
const el = $('#svelte-sass');
const classes = el.attr('class').split(' ');
const scopedClass = classes.find((name) => /^s-[A-Za-z0-9-]+/.test(name));
const scopedClass = classes.find((name) => name !== 'svelte-sass' && /^svelte-[A-Za-z0-9-]+/.test(name));

// 1. check HTML
expect(el.attr('class')).to.include('svelte-sass');
Expand All @@ -242,7 +248,7 @@ describe('CSS', function () {
it('<style lang="scss">', async () => {
const el = $('#svelte-scss');
const classes = el.attr('class').split(' ');
const scopedClass = classes.find((name) => /^s-[A-Za-z0-9-]+/.test(name));
const scopedClass = classes.find((name) => name !== 'svelte-scss' && /^svelte-[A-Za-z0-9-]+/.test(name));

// 1. check HTML
expect(el.attr('class')).to.include('svelte-scss');
Expand Down Expand Up @@ -273,11 +279,6 @@ describe('CSS', function () {
expect((await fixture.fetch(href)).status).to.equal(200);
});

it('resolves CSS in src/', async () => {
const href = $('link[href$="linked.css"]').attr('href');
expect((await fixture.fetch(href)).status).to.equal(200);
});

// Skipped until upstream fix lands
// Our fix: https://github.com/withastro/astro/pull/2106
// OG Vite PR: https://github.com/vitejs/vite/pull/5940
Expand Down
4 changes: 1 addition & 3 deletions packages/astro/test/astro-css-bundling-import.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ describe('CSS Bundling (ESM import)', () => {
before(async () => {
fixture = await loadFixture({
projectRoot: './fixtures/astro-css-bundling-import/',
buildOptions: { legacyBuild: true } // TODO make this test work without legacyBuild
});
await fixture.build();
});
Expand All @@ -35,8 +34,7 @@ describe('CSS Bundling (ESM import)', () => {
expect(css.indexOf('p{color:green}')).to.be.greaterThan(css.indexOf('p{color:#00f}'));
});

// TODO: re-enable this
it.skip('no empty CSS files', async () => {
it('no empty CSS files', async () => {
for (const page of ['/page-1/index.html', '/page-2/index.html']) {
const html = await fixture.readFile(page);
const $ = cheerio.load(html);
Expand Down
Loading

0 comments on commit 8f08197

Please sign in to comment.