Output assets to _astro directory (#5772)

* WIP: emit assets to _astro * chore: better _astro handling * refactor: emit server assets to `chunks/` * chore: update /asset tests * test: add explicit build output tests * fix: update image to emit to configured asset path * chore: update changeset * chore: update image tests * chore: update image tests * test: update css test * test: update bundling test * test: update tests * Update packages/astro/src/@types/astro.ts Co-authored-by: Chris Swithinbank <[email protected]> * Update packages/astro/src/@types/astro.ts Co-authored-by: Chris Swithinbank <[email protected]> * Update packages/astro/src/@types/astro.ts Co-authored-by: Chris Swithinbank <[email protected]> * chore: add clarifying comment Co-authored-by: Nate Moore <[email protected]> Co-authored-by: Chris Swithinbank <[email protected]>
withastro · Jan 9, 2023 · f8f4d49 · f8f4d49
1 parent 7572f74
commit f8f4d49
Show file tree

Hide file tree

Showing 32 changed files with 367 additions and 135 deletions.
diff --git a/.changeset/lovely-worms-invite.md b/.changeset/lovely-worms-invite.md
@@ -1,8 +1,10 @@
 ---
 '@astrojs/deno': major
 '@astrojs/netlify': major
-'@astrojs/image': minor
+'@astrojs/image': major
 'astro': major
 ---
 
-Builds chunks into the `assets` folder. This simplifies configuring immutable caching with your adapter provider as all files are now in the same `assets` folder.
+**Breaking Change**: client assets are built to an `_astro` directory rather than the previous `assets` directory. This setting can now be controlled by the new `build` configuration option named `assets`.
+
+This should simplify configuring immutable caching with your adapter provider as all files are now in the same `_astro` directory.
diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -586,6 +586,25 @@ export interface AstroUserConfig {
 		 * ```
 		 */
 		server?: string;
+		/**
+		 * @docs
+		 * @name build.assets
+		 * @type {string}
+		 * @default `'_astro'`
+		 * @see outDir
+		 * @version 2.0.0
+		 * @description
+		 * Specifies the directory in the build output where Astro-generated assets (bundled JS and CSS for example) should live.
+		 *
+		 * ```js
+		 * {
+		 *   build: {
+		 *     assets: '_custom'
+		 *   }
+		 * }
+		 * ```
+		 */
+		assets?: string;
 		/**
 		 * @docs
 		 * @name build.serverEntry

diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts
@@ -10,9 +10,10 @@ import {
 	BuildInternals,
 	createBuildInternals,
 	eachPrerenderedPageData,
+	isHoistedScript,
 } from '../../core/build/internal.js';
-import { emptyDir, removeDir } from '../../core/fs/index.js';
-import { prependForwardSlash } from '../../core/path.js';
+import { emptyDir, removeDir, removeEmptyDirs } from '../../core/fs/index.js';
+import { appendForwardSlash, prependForwardSlash } from '../../core/path.js';
 import { isModeServerWithNoAdapter } from '../../core/util.js';
 import { runHookBuildSetup } from '../../integrations/index.js';
 import { PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
@@ -133,8 +134,10 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
 				input: [],
 				output: {
 					format: 'esm',
-					chunkFileNames: 'chunks/[name].[hash].mjs',
-					assetFileNames: 'assets/[name].[hash][extname]',
+					// Server chunks can't go in the assets (_astro) folder
+					// We need to keep these separate
+					chunkFileNames: `chunks/[name].[hash].mjs`,
+					assetFileNames: `${settings.config.build.assets}/[name].[hash][extname]`,
 					...viteConfig.build?.rollupOptions?.output,
 					entryFileNames: opts.buildConfig.serverEntry,
 				},
@@ -212,9 +215,9 @@ async function clientBuild(
 				input: Array.from(input),
 				output: {
 					format: 'esm',
-					entryFileNames: 'assets/[name].[hash].js',
-					chunkFileNames: 'assets/chunks/[name].[hash].js',
-					assetFileNames: 'assets/[name].[hash][extname]',
+					entryFileNames: `${settings.config.build.assets}/[name].[hash].js`,
+					chunkFileNames: `${settings.config.build.assets}/[name].[hash].js`,
+					assetFileNames: `${settings.config.build.assets}/[name].[hash][extname]`,
 					...viteConfig.build?.rollupOptions?.output,
 				},
 				preserveEntrySignatures: 'exports-only',
@@ -285,25 +288,8 @@ async function cleanStaticOutput(opts: StaticBuildOptions, internals: BuildInter
 				await fs.promises.writeFile(url, value, { encoding: 'utf8' });
 			})
 		);
-		// Map directories heads from the .mjs files
-		const directories: Set<string> = new Set();
-		files.forEach((i) => {
-			const splitFilePath = i.split(path.sep);
-			// If the path is more than just a .mjs filename itself
-			if (splitFilePath.length > 1) {
-				directories.add(splitFilePath[0]);
-			}
-		});
-		// Attempt to remove only those folders which are empty
-		await Promise.all(
-			Array.from(directories).map(async (filename) => {
-				const url = new URL(filename, out);
-				const folder = await fs.promises.readdir(url);
-				if (!folder.length) {
-					await fs.promises.rm(url, { recursive: true, force: true });
-				}
-			})
-		);
+
+		removeEmptyDirs(out);
 	}
 }
 
@@ -321,28 +307,10 @@ async function cleanServerOutput(opts: StaticBuildOptions) {
 				await fs.promises.rm(url);
 			})
 		);
-		// Map directories heads from the .mjs files
-		const directories: Set<string> = new Set();
-		files.forEach((i) => {
-			const splitFilePath = i.split(path.sep);
-			// If the path is more than just a .mjs filename itself
-			if (splitFilePath.length > 1) {
-				directories.add(splitFilePath[0]);
-			}
-		});
-		// Attempt to remove only those folders which are empty
-		await Promise.all(
-			Array.from(directories).map(async (filename) => {
-				const url = new URL(filename, out);
-				const dir = await glob(fileURLToPath(url));
-				// Do not delete chunks/ directory!
-				if (filename === 'chunks') return;
-				if (!dir.length) {
-					await fs.promises.rm(url, { recursive: true, force: true });
-				}
-			})
-		);
+
+		removeEmptyDirs(out);
 	}
+
 	// Clean out directly if the outDir is outside of root
 	if (out.toString() !== opts.settings.config.outDir.toString()) {
 		// Copy assets before cleaning directory if outside root
@@ -374,22 +342,23 @@ async function ssrMoveAssets(opts: StaticBuildOptions) {
 	const serverRoot =
 		opts.settings.config.output === 'static' ? opts.buildConfig.client : opts.buildConfig.server;
 	const clientRoot = opts.buildConfig.client;
-	const serverAssets = new URL('./assets/', serverRoot);
-	const clientAssets = new URL('./assets/', clientRoot);
-	const files = await glob('assets/**/*', {
-		cwd: fileURLToPath(serverRoot),
+	const assets = opts.settings.config.build.assets;
+	const serverAssets = new URL(`./${assets}/`, appendForwardSlash(serverRoot.toString()));
+	const clientAssets = new URL(`./${assets}/`, appendForwardSlash(clientRoot.toString()));
+	const files = await glob(`**/*`, {
+		cwd: fileURLToPath(serverAssets),
 	});
 
 	if (files.length > 0) {
 		// Make the directory
 		await fs.promises.mkdir(clientAssets, { recursive: true });
 		await Promise.all(
 			files.map(async (filename) => {
-				const currentUrl = new URL(filename, serverRoot);
-				const clientUrl = new URL(filename, clientRoot);
+				const currentUrl = new URL(filename, appendForwardSlash(serverAssets.toString()));
+				const clientUrl = new URL(filename, appendForwardSlash(clientAssets.toString()));
 				return fs.promises.rename(currentUrl, clientUrl);
 			})
 		);
-		removeDir(serverAssets);
+		removeEmptyDirs(serverAssets);
 	}
 }
diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
@@ -23,6 +23,7 @@ const ASTRO_CONFIG_DEFAULTS: AstroUserConfig & any = {
 		format: 'directory',
 		client: './dist/client/',
 		server: './dist/server/',
+		assets: '_astro',
 		serverEntry: 'entry.mjs',
 	},
 	server: {
@@ -102,6 +103,7 @@ export const AstroConfigSchema = z.object({
 				.optional()
 				.default(ASTRO_CONFIG_DEFAULTS.build.server)
 				.transform((val) => new URL(val)),
+			assets: z.string().optional().default(ASTRO_CONFIG_DEFAULTS.build.assets),
 			serverEntry: z.string().optional().default(ASTRO_CONFIG_DEFAULTS.build.serverEntry),
 		})
 		.optional()
@@ -246,6 +248,7 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: URL) {
 					.optional()
 					.default(ASTRO_CONFIG_DEFAULTS.build.server)
 					.transform((val) => new URL(val, fileProtocolRoot)),
+				assets: z.string().optional().default(ASTRO_CONFIG_DEFAULTS.build.assets),
 				serverEntry: z.string().optional().default(ASTRO_CONFIG_DEFAULTS.build.serverEntry),
 			})
 			.optional()

diff --git a/packages/astro/src/core/fs/index.ts b/packages/astro/src/core/fs/index.ts
@@ -1,6 +1,7 @@
 import fs from 'fs';
 import path from 'path';
 import { fileURLToPath } from 'url';
+import { appendForwardSlash } from '../path.js';
 
 const isWindows = process.platform === 'win32';
 
@@ -10,6 +11,24 @@ export function removeDir(_dir: URL): void {
 	fs.rmSync(dir, { recursive: true, force: true, maxRetries: 3 });
 }
 
+export function removeEmptyDirs(root: URL): void {
+	const dir = fileURLToPath(root);
+	if (!fs.statSync(dir).isDirectory()) return;
+	let files = fs.readdirSync(dir);
+
+	if (files.length > 0) {
+		files.map(file => {
+			const url = new URL(`./${file}`, appendForwardSlash(root.toString()));
+			removeEmptyDirs(url);
+		})
+		files = fs.readdirSync(dir);
+	}
+
+	if (files.length === 0) {
+		fs.rmdirSync(dir);
+	}
+}
+
 export function emptyDir(_dir: URL, skip?: Set<string>): void {
 	const dir = fileURLToPath(_dir);
 	if (!fs.existsSync(dir)) return undefined;

diff --git a/packages/astro/test/0-css.test.js b/packages/astro/test/0-css.test.js
@@ -29,7 +29,7 @@ describe('CSS', function () {
 			// get bundled CSS (will be hashed, hence DOM query)
 			html = await fixture.readFile('/index.html');
 			$ = cheerio.load(html);
-			const bundledCSSHREF = $('link[rel=stylesheet][href^=/assets/]').attr('href');
+			const bundledCSSHREF = $('link[rel=stylesheet][href^=/_astro/]').attr('href');
 			bundledCSS = (await fixture.readFile(bundledCSSHREF.replace(/^\/?/, '/')))
 				.replace(/\s/g, '')
 				.replace('/n', '');
@@ -364,7 +364,7 @@ describe('CSS', function () {
 		});
 
 		it('remove unused styles from client:load components', async () => {
-			const bundledAssets = await fixture.readdir('./assets');
+			const bundledAssets = await fixture.readdir('./_astro');
 			// SvelteDynamic styles is already included in the main page css asset
 			const unusedCssAsset = bundledAssets.find((asset) => /SvelteDynamic\..*\.css/.test(asset));
 			expect(unusedCssAsset, 'Found unused style ' + unusedCssAsset).to.be.undefined;

diff --git a/packages/astro/test/astro-css-bundling.test.js b/packages/astro/test/astro-css-bundling.test.js
@@ -5,9 +5,9 @@ import { loadFixture } from './test-utils.js';
 // note: the hashes should be deterministic, but updating the file contents will change hashes
 // be careful not to test that the HTML simply contains CSS, because it always will! filename and quanity matter here (bundling).
 const EXPECTED_CSS = {
-	'/index.html': ['/assets/'], // don’t match hashes, which change based on content
-	'/one/index.html': ['/assets/'],
-	'/two/index.html': ['/assets/'],
+	'/index.html': ['/_astro/'], // don’t match hashes, which change based on content
+	'/one/index.html': ['/_astro/'],
+	'/two/index.html': ['/_astro/'],
 };
 const UNEXPECTED_CSS = [
 	'/src/components/nav.css',
@@ -61,12 +61,12 @@ describe('CSS Bundling', function () {
 		});
 
 		it('there are 4 css files', async () => {
-			const dir = await fixture.readdir('/assets');
+			const dir = await fixture.readdir('/_astro');
 			expect(dir).to.have.a.lengthOf(4);
 		});
 
 		it('CSS includes hashes', async () => {
-			const [firstFound] = await fixture.readdir('/assets');
+			const [firstFound] = await fixture.readdir('/_astro');
 			expect(firstFound).to.match(/[a-z]+\.[0-9a-z]{8}\.css/);
 		});
 	});

diff --git a/packages/astro/test/astro-dynamic.test.js b/packages/astro/test/astro-dynamic.test.js
@@ -75,6 +75,6 @@ describe('Dynamic components subpath', () => {
 		expect($('astro-island').html()).to.equal('');
 		// test 2: has component url
 		const attr = $('astro-island').attr('component-url');
-		expect(attr).to.include(`blog/assets/PersistentCounter`);
+		expect(attr).to.include(`blog/_astro/PersistentCounter`);
 	});
 });
diff --git a/packages/astro/test/astro-envs.test.js b/packages/astro/test/astro-envs.test.js
@@ -52,7 +52,7 @@ describe('Environment Variables', () => {
 		});
 
 		it('includes public env in client-side JS', async () => {
-			let dirs = await fixture.readdir('/assets');
+			let dirs = await fixture.readdir('/_astro');
 			console.log(dirs);
 			let found = false;
 
@@ -62,7 +62,7 @@ describe('Environment Variables', () => {
 			await Promise.all(
 				dirs.map(async (path) => {
 					if (path.endsWith('.js')) {
-						let js = await fixture.readFile(`/assets/${path}`);
+						let js = await fixture.readFile(`/_astro/${path}`);
 						if (js.includes('BLUE_BAYOU')) {
 							found = true;
 						}

diff --git a/packages/astro/test/astro-scripts.test.js b/packages/astro/test/astro-scripts.test.js
@@ -73,7 +73,7 @@ describe('Scripts (hoisted and not)', () => {
 
 			// test 2: inside assets
 			let entryURL = $('script').attr('src');
-			expect(entryURL.includes('assets/')).to.equal(true);
+			expect(entryURL.includes('_astro/')).to.equal(true);
 		});
 
 		it('External page using non-hoist scripts that are not modules are built standalone', async () => {
@@ -85,7 +85,7 @@ describe('Scripts (hoisted and not)', () => {
 
 			// test 2: inside assets
 			let entryURL = $('script').attr('src');
-			expect(entryURL.includes('assets/')).to.equal(true);
+			expect(entryURL.includes('_astro/')).to.equal(true);
 		});
 
 		it('Scripts added via Astro.glob are hoisted', async () => {