Merge branch 'main' into perf/server-islands-json

.changeset/heavy-yaks-brake.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/underscore-redirects': minor
+---
+
+Updates how the output is determined in `createRedirectsFromAstroRoutes`. Since `v0.5.0`, the output would use the `buildOutput` property and `config.output` as a fallback. It no longer uses this fallback.

.changeset/rich-terms-sit.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug where the logged output path does not match the actual output path when using `build.format: 'preserve'`

.changeset/tiny-plums-crash.md

@@ -0,0 +1,40 @@
+---
+'@astrojs/underscore-redirects': minor
+---
+
+Updates the input requirements of `createRedirectsFromAstroRoutes`:
+
+- `routeToDynamicTargetMap` keys are `IntegrationResolvedRoute` instead of `IntegrationRouteData` (obtained from the `astro:routes:resolved` hook)
+- There's a new `assets` property, that can be obtained from the `astro:build:done` hook
+
+```js
+function myIntegration() {
+    let routes
+    let buildOutput
+    let config
+
+    return {
+        name: "my-integration",
+        hooks: {
+            "astro:routes:resolved": (params) => {
+                routes = params.routes
+            },
+            "astro:config:done": (params) => {
+                buildOutput = params.buildOutput
+                config = params.config
+            },
+            "astro:build:done": (params) => {
+                const redirects = createRedirectsFromAstroRoutes({
+                    config,
+                    buildOutput,
+                    routeToDynamicTargetMap: new Map(
+                        routes.map(route => [route, ''])
+                    ),
+                    dir: params.dir,
+                    assets: params.assets
+                })
+            }
+        }
+    }
+}
+```

packages/astro/src/core/build/generate.ts

@@ -181,7 +181,7 @@ async function generatePage(
 		const timeStart = performance.now();
 		pipeline.logger.debug('build', `Generating: ${path}`);
 
-		const filePath = getOutputFilename(config, path, pageData.route.type);
+		const filePath = getOutputFilename(config, path, pageData.route);
 		const lineIcon =
 			(index === paths.length - 1 && !isConcurrent) || paths.length === 1 ? '└─' : '├─';
 

packages/astro/src/core/util.ts

@@ -3,7 +3,7 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import type { AstroSettings } from '../types/astro.js';
 import type { AstroConfig } from '../types/public/config.js';
-import type { RouteType } from '../types/public/internal.js';
+import type { RouteData } from '../types/public/internal.js';
 import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from './constants.js';
 import { removeQueryString, removeTrailingForwardSlash, slash } from './path.js';
 
@@ -43,8 +43,8 @@ const STATUS_CODE_PAGES = new Set(['/404', '/500']);
  * Handles both "/foo" and "foo" `name` formats.
  * Handles `/404` and `/` correctly.
  */
-export function getOutputFilename(astroConfig: AstroConfig, name: string, type: RouteType) {
-	if (type === 'endpoint') {
+export function getOutputFilename(astroConfig: AstroConfig, name: string, routeData: RouteData) {
+	if (routeData.type === 'endpoint') {
 		return name;
 	}
 	if (name === '/' || name === '') {
@@ -53,6 +53,9 @@ export function getOutputFilename(astroConfig: AstroConfig, name: string, type:
 	if (astroConfig.build.format === 'file' || STATUS_CODE_PAGES.has(name)) {
 		return `${removeTrailingForwardSlash(name || 'index')}.html`;
 	}
+	if (astroConfig.build.format === 'preserve' && !routeData.isIndex) {
+		return `${removeTrailingForwardSlash(name || 'index')}.html`;
+	}
 	return path.posix.join(name, 'index.html');
 }
 

packages/astro/test/astro-pageDirectoryUrl.test.js

@@ -1,11 +1,14 @@
 import assert from 'node:assert/strict';
+import { Writable } from 'node:stream';
 import { before, describe, it } from 'node:test';
+import { Logger } from '../dist/core/logger/core.js';
 import { loadFixture } from './test-utils.js';
 
 describe('build format', () => {
 	describe('build.format: file', () => {
 		/** @type {import('./test-utils.js').Fixture} */
 		let fixture;
+		const logs = [];
 
 		before(async () => {
 			fixture = await loadFixture({
@@ -14,19 +17,41 @@ describe('build format', () => {
 					format: 'file',
 				},
 			});
-			await fixture.build();
+			await fixture.build({
+				logger: new Logger({
+					level: 'info',
+					dest: new Writable({
+						objectMode: true,
+						write(event, _, callback) {
+							logs.push(event);
+							callback();
+						},
+					}),
+				}),
+			});
 		});
 
 		it('outputs', async () => {
 			assert.ok(await fixture.readFile('/client.html'));
 			assert.ok(await fixture.readFile('/nested-md.html'));
 			assert.ok(await fixture.readFile('/nested-astro.html'));
 		});
+
+		it('logs correct output paths', () => {
+			assert.ok(logs.find((log) => log.level === 'info' && log.message.includes('/client.html')));
+			assert.ok(
+				logs.find((log) => log.level === 'info' && log.message.includes('/nested-md.html')),
+			);
+			assert.ok(
+				logs.find((log) => log.level === 'info' && log.message.includes('/nested-astro.html')),
+			);
+		});
 	});
 
 	describe('build.format: preserve', () => {
 		/** @type {import('./test-utils.js').Fixture} */
 		let fixture;
+		const logs = [];
 
 		before(async () => {
 			fixture = await loadFixture({
@@ -35,13 +60,36 @@ describe('build format', () => {
 					format: 'preserve',
 				},
 			});
-			await fixture.build();
+			await fixture.build({
+				logger: new Logger({
+					level: 'info',
+					dest: new Writable({
+						objectMode: true,
+						write(event, _, callback) {
+							logs.push(event);
+							callback();
+						},
+					}),
+				}),
+			});
 		});
 
 		it('outputs', async () => {
 			assert.ok(await fixture.readFile('/client.html'));
 			assert.ok(await fixture.readFile('/nested-md/index.html'));
 			assert.ok(await fixture.readFile('/nested-astro/index.html'));
 		});
+
+		it('logs correct output paths', () => {
+			assert.ok(logs.find((log) => log.level === 'info' && log.message.includes('/client.html')));
+			assert.ok(
+				logs.find((log) => log.level === 'info' && log.message.includes('/nested-md/index.html')),
+			);
+			assert.ok(
+				logs.find(
+					(log) => log.level === 'info' && log.message.includes('/nested-astro/index.html'),
+				),
+			);
+		});
 	});
 });

packages/underscore-redirects/src/astro.ts

@@ -1,10 +1,15 @@
 import { posix } from 'node:path';
-import type { AstroConfig, IntegrationRouteData, ValidRedirectStatus } from 'astro';
+import type {
+	AstroConfig,
+	HookParameters,
+	IntegrationResolvedRoute,
+	ValidRedirectStatus,
+} from 'astro';
 import { Redirects } from './redirects.js';
 
 const pathJoin = posix.join;
 
-function getRedirectStatus(route: IntegrationRouteData): ValidRedirectStatus {
+function getRedirectStatus(route: IntegrationResolvedRoute): ValidRedirectStatus {
 	if (typeof route.redirect === 'object') {
 		return route.redirect.status;
 	}
@@ -16,9 +21,10 @@ interface CreateRedirectsFromAstroRoutesParams {
 	/**
 	 * Maps a `RouteData` to a dynamic target
 	 */
-	routeToDynamicTargetMap: Map<IntegrationRouteData, string>;
+	routeToDynamicTargetMap: Map<IntegrationResolvedRoute, string>;
 	dir: URL;
 	buildOutput: 'static' | 'server';
+	assets: HookParameters<'astro:build:done'>['assets'];
 }
 
 /**
@@ -29,18 +35,18 @@ export function createRedirectsFromAstroRoutes({
 	routeToDynamicTargetMap,
 	dir,
 	buildOutput,
+	assets,
 }: CreateRedirectsFromAstroRoutesParams) {
 	const base =
 		config.base && config.base !== '/'
 			? config.base.endsWith('/')
 				? config.base.slice(0, -1)
 				: config.base
 			: '';
-	// TODO: the use of `config.output` is deprecated. We need to update the adapters that use this package to pass the new buildOutput
-	const output = buildOutput ?? config.output;
 	const _redirects = new Redirects();
 
 	for (const [route, dynamicTarget = ''] of routeToDynamicTargetMap) {
+		const distURL = assets.get(route.pattern);
 		// A route with a `pathname` is as static route.
 		if (route.pathname) {
 			if (route.redirect) {
@@ -57,13 +63,13 @@ export function createRedirectsFromAstroRoutes({
 			}
 
 			// If this is a static build we don't want to add redirects to the HTML file.
-			if (output === 'static') {
+			if (buildOutput === 'static') {
 				continue;
-			} else if (route.distURL) {
+			} else if (distURL) {
 				_redirects.add({
 					dynamic: false,
 					input: `${base}${route.pathname}`,
-					target: prependForwardSlash(route.distURL.toString().replace(dir.toString(), '')),
+					target: prependForwardSlash(distURL.toString().replace(dir.toString(), '')),
 					status: 200,
 					weight: 2,
 				});
@@ -76,7 +82,7 @@ export function createRedirectsFromAstroRoutes({
 					weight: 2,
 				});
 
-				if (route.route === '/404') {
+				if (route.pattern === '/404') {
 					_redirects.add({
 						dynamic: true,
 						input: '/*',
@@ -92,7 +98,7 @@ export function createRedirectsFromAstroRoutes({
 			const pattern = generateDynamicPattern(route);
 
 			// This route was prerendered and should be forwarded to the HTML file.
-			if (route.distURL) {
+			if (distURL) {
 				const targetRoute = route.redirectRoute ?? route;
 				const targetPattern = generateDynamicPattern(targetRoute);
 				let target = targetPattern;
@@ -128,7 +134,7 @@ export function createRedirectsFromAstroRoutes({
  * /team/articles/*
  * With stars replacing spread and :id syntax replacing [id]
  */
-function generateDynamicPattern(route: IntegrationRouteData) {
+function generateDynamicPattern(route: IntegrationResolvedRoute) {
 	const pattern =
 		'/' +
 		route.segments

packages/underscore-redirects/test/astro.test.js

@@ -3,28 +3,24 @@ import { describe, it } from 'node:test';
 import { createRedirectsFromAstroRoutes } from '../dist/index.js';
 
 describe('Astro', () => {
-	const serverConfig = {
-		output: 'server',
-		build: { format: 'directory' },
-	};
-
 	it('Creates a Redirects object from routes', () => {
 		const routeToDynamicTargetMap = new Map(
 			Array.from([
-				[
-					{ pathname: '/', distURL: new URL('./index.html', import.meta.url), segments: [] },
-					'./.adapter/dist/entry.mjs',
-				],
-				[
-					{ pathname: '/one', distURL: new URL('./one/index.html', import.meta.url), segments: [] },
-					'./.adapter/dist/entry.mjs',
-				],
+				[{ pattern: '/', pathname: '/', segments: [] }, './.adapter/dist/entry.mjs'],
+				[{ pattern: '/one', pathname: '/one', segments: [] }, './.adapter/dist/entry.mjs'],
 			]),
 		);
 		const _redirects = createRedirectsFromAstroRoutes({
-			config: serverConfig,
+			config: {
+				build: { format: 'directory' },
+			},
 			routeToDynamicTargetMap,
 			dir: new URL(import.meta.url),
+			buildOutput: 'server',
+			assets: new Map([
+				['/', new URL('./index.html', import.meta.url)],
+				['/one', new URL('./one/index.html', import.meta.url)],
+			]),
 		});
 
 		assert.equal(_redirects.definitions.length, 2);