From fd12a26ac6012c6b8a26f5a178e1bb46092a1806 Mon Sep 17 00:00:00 2001 From: lam eu ler <27113373+lameuler@users.noreply.github.com> Date: Wed, 8 Jan 2025 21:57:53 +0800 Subject: [PATCH 1/2] fix output path logging for build.format: preserve (#12918) * fix output path logging for build.format: preserve * add tests for output logging * code format --- .changeset/rich-terms-sit.md | 5 ++ packages/astro/src/core/build/generate.ts | 2 +- packages/astro/src/core/util.ts | 9 ++-- .../astro/test/astro-pageDirectoryUrl.test.js | 52 ++++++++++++++++++- 4 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 .changeset/rich-terms-sit.md diff --git a/.changeset/rich-terms-sit.md b/.changeset/rich-terms-sit.md new file mode 100644 index 000000000000..40258bf952c3 --- /dev/null +++ b/.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'` diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 3c27bf6a7bd6..ca19b5b63bf0 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/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 ? '└─' : '├─'; diff --git a/packages/astro/src/core/util.ts b/packages/astro/src/core/util.ts index dd663de83952..1bd53d779b1a 100644 --- a/packages/astro/src/core/util.ts +++ b/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'); } diff --git a/packages/astro/test/astro-pageDirectoryUrl.test.js b/packages/astro/test/astro-pageDirectoryUrl.test.js index 76e224b07466..9b454a736723 100644 --- a/packages/astro/test/astro-pageDirectoryUrl.test.js +++ b/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,7 +17,18 @@ 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 () => { @@ -22,11 +36,22 @@ describe('build format', () => { 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,7 +60,18 @@ 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 () => { @@ -43,5 +79,17 @@ describe('build format', () => { 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'), + ), + ); + }); }); }); From 3caa337f0ba917ad677fd8438b7045abc5d29e1c Mon Sep 17 00:00:00 2001 From: Florian Lefebvre Date: Wed, 8 Jan 2025 15:23:42 +0100 Subject: [PATCH 2/2] feat(underscore-redirects): update API to support new hook (#12924) --- .changeset/heavy-yaks-brake.md | 5 +++ .changeset/tiny-plums-crash.md | 40 +++++++++++++++++++ packages/underscore-redirects/src/astro.ts | 28 ++++++++----- .../underscore-redirects/test/astro.test.js | 24 +++++------ 4 files changed, 72 insertions(+), 25 deletions(-) create mode 100644 .changeset/heavy-yaks-brake.md create mode 100644 .changeset/tiny-plums-crash.md diff --git a/.changeset/heavy-yaks-brake.md b/.changeset/heavy-yaks-brake.md new file mode 100644 index 000000000000..50f2a1f88e3f --- /dev/null +++ b/.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. diff --git a/.changeset/tiny-plums-crash.md b/.changeset/tiny-plums-crash.md new file mode 100644 index 000000000000..815a4a354268 --- /dev/null +++ b/.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 + }) + } + } + } +} +``` \ No newline at end of file diff --git a/packages/underscore-redirects/src/astro.ts b/packages/underscore-redirects/src/astro.ts index b76e1b52a672..bd3e3b2ea346 100644 --- a/packages/underscore-redirects/src/astro.ts +++ b/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; + routeToDynamicTargetMap: Map; dir: URL; buildOutput: 'static' | 'server'; + assets: HookParameters<'astro:build:done'>['assets']; } /** @@ -29,6 +35,7 @@ export function createRedirectsFromAstroRoutes({ routeToDynamicTargetMap, dir, buildOutput, + assets, }: CreateRedirectsFromAstroRoutesParams) { const base = config.base && config.base !== '/' @@ -36,11 +43,10 @@ export function createRedirectsFromAstroRoutes({ ? 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 diff --git a/packages/underscore-redirects/test/astro.test.js b/packages/underscore-redirects/test/astro.test.js index 71ee13f339c4..6a4944dc907a 100644 --- a/packages/underscore-redirects/test/astro.test.js +++ b/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);