diff --git a/.changeset/empty-crews-scream.md b/.changeset/empty-crews-scream.md new file mode 100644 index 0000000000..c52237ab8c --- /dev/null +++ b/.changeset/empty-crews-scream.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where image and server islands routes would not be passed to the `astro:routes:resolved` hook during builds diff --git a/.changeset/twenty-keys-divide.md b/.changeset/twenty-keys-divide.md new file mode 100644 index 0000000000..9e84058674 --- /dev/null +++ b/.changeset/twenty-keys-divide.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a case where failing content entries in `astro check` would not be surfaced diff --git a/packages/astro/src/assets/endpoint/config.ts b/packages/astro/src/assets/endpoint/config.ts index cce5cde5fc..b59bdc626b 100644 --- a/packages/astro/src/assets/endpoint/config.ts +++ b/packages/astro/src/assets/endpoint/config.ts @@ -16,17 +16,6 @@ export function injectImageEndpoint( manifest.routes.unshift(getImageEndpointData(settings, mode, cwd)); } -export function ensureImageEndpointRoute( - settings: AstroSettings, - manifest: ManifestData, - mode: 'dev' | 'build', - cwd?: string, -) { - if (!manifest.routes.some((route) => route.route === settings.config.image.endpoint.route)) { - manifest.routes.unshift(getImageEndpointData(settings, mode, cwd)); - } -} - function getImageEndpointData( settings: AstroSettings, mode: 'dev' | 'build', diff --git a/packages/astro/src/cli/check/index.ts b/packages/astro/src/cli/check/index.ts index c93e3b2f4c..b7e03aa30e 100644 --- a/packages/astro/src/cli/check/index.ts +++ b/packages/astro/src/cli/check/index.ts @@ -31,11 +31,7 @@ export async function check(flags: Flags) { // NOTE: In the future, `@astrojs/check` can expose a `before lint` hook so that this works during `astro check --watch` too. // For now, we run this once as usually `astro check --watch` is ran alongside `astro dev` which also calls `astro sync`. const { default: sync } = await import('../../core/sync/index.js'); - try { - await sync(flagsToAstroInlineConfig(flags)); - } catch (_) { - return process.exit(1); - } + await sync(flagsToAstroInlineConfig(flags)); } const { check: checker, parseArgsAsCheckConfig } = checkPackage; diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index d23383133e..a3d08bb642 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -20,7 +20,8 @@ import { } from '../path.js'; import { RenderContext } from '../render-context.js'; import { createAssetLink } from '../render/ssr-element.js'; -import { createDefaultRoutes, injectDefaultRoutes } from '../routing/default.js'; +import { ensure404Route } from '../routing/astro-designed-error-pages.js'; +import { createDefaultRoutes } from '../routing/default.js'; import { matchRoute } from '../routing/match.js'; import { AppPipeline } from './pipeline.js'; @@ -88,9 +89,12 @@ export class App { constructor(manifest: SSRManifest, streaming = true) { this.#manifest = manifest; - this.#manifestData = injectDefaultRoutes(manifest, { + this.#manifestData = { routes: manifest.routes.map((route) => route.routeData), - }); + }; + // This is necessary to allow running middlewares for 404 in SSR. There's special handling + // to return the host 404 if the user doesn't provide a custom 404 + ensure404Route(this.#manifestData); this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base); this.#pipeline = this.#createPipeline(this.#manifestData, streaming); this.#adapterLogger = new AstroIntegrationLogger( diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 68cf650acd..bcc7fb96d5 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -3,7 +3,6 @@ import { performance } from 'node:perf_hooks'; import { fileURLToPath } from 'node:url'; import { blue, bold, green } from 'kleur/colors'; import type * as vite from 'vite'; -import { injectImageEndpoint } from '../../assets/endpoint/config.js'; import { telemetry } from '../../events/index.js'; import { eventCliSession } from '../../events/session.js'; import { @@ -24,7 +23,6 @@ import type { Logger } from '../logger/core.js'; import { levels, timerMessage } from '../logger/core.js'; import { apply as applyPolyfill } from '../polyfill.js'; import { createRouteManifest } from '../routing/index.js'; -import { getServerIslandRouteData } from '../server-islands/endpoint.js'; import { clearContentLayerCache } from '../sync/index.js'; import { ensureProcessNodeEnv } from '../util.js'; import { collectPagesData } from './page-data.js'; @@ -123,10 +121,6 @@ class AstroBuilder { this.manifest = await createRouteManifest({ settings: this.settings }, this.logger); - if (this.settings.buildOutput === 'server') { - injectImageEndpoint(this.settings, this.manifest, 'build'); - } - await runHookConfigDone({ settings: this.settings, logger: logger, command: 'build' }); // If we're building for the server, we need to ensure that an adapter is installed. @@ -239,8 +233,7 @@ class AstroBuilder { pages: pageNames, routes: Object.values(allPages) .flat() - .map((pageData) => pageData.route) - .concat(hasServerIslands ? getServerIslandRouteData(this.settings.config) : []), + .map((pageData) => pageData.route), logging: this.logger, }); diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index c36fe0d57b..fc33067d07 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -4,6 +4,7 @@ import type { AllPagesData } from './types.js'; import * as colors from 'kleur/colors'; import { debug } from '../logger/core.js'; +import { DEFAULT_COMPONENTS } from '../routing/default.js'; import { makePageDataKey } from './plugins/util.js'; export interface CollectPagesDataOptions { @@ -29,6 +30,11 @@ export function collectPagesData(opts: CollectPagesDataOptions): CollectPagesDat // and is then cached across all future SSR builds. In the past, we've had trouble // with parallelized builds without guaranteeing that this is called first. for (const route of manifest.routes) { + // There's special handling in SSR + if (DEFAULT_COMPONENTS.some((component) => route.component === component)) { + continue; + } + // Generate a unique key to identify each page in the build process. const key = makePageDataKey(route.route, route.component); // static route: diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index f0589868f7..e220292ebf 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -14,6 +14,7 @@ import type { } from '../../app/types.js'; import { encodeKey } from '../../encryption.js'; import { fileExtension, joinPaths, prependForwardSlash } from '../../path.js'; +import { DEFAULT_COMPONENTS } from '../../routing/default.js'; import { serializeRouteData } from '../../routing/index.js'; import { addRollupInput } from '../add-rollup-input.js'; import { getOutFile, getOutFolder } from '../common.js'; @@ -172,6 +173,22 @@ function buildManifest( } }; + // Default components follow a special flow during build. We prevent their processing earlier + // in the build. As a result, they are not present on `internals.pagesByKeys` and not serialized + // in the manifest file. But we need them in the manifest, so we handle them here + for (const route of opts.manifest.routes) { + if (!DEFAULT_COMPONENTS.find((component) => route.component === component)) { + continue; + } + routes.push({ + file: '', + links: [], + scripts: [], + styles: [], + routeData: serializeRouteData(route, settings.config.trailingSlash), + }); + } + for (const route of opts.manifest.routes) { if (!route.prerender) continue; if (!route.pathname) continue; diff --git a/packages/astro/src/core/dev/container.ts b/packages/astro/src/core/dev/container.ts index 063681f271..3484227afb 100644 --- a/packages/astro/src/core/dev/container.ts +++ b/packages/astro/src/core/dev/container.ts @@ -7,7 +7,6 @@ import * as vite from 'vite'; import { runHookConfigDone, runHookConfigSetup, - runHookRoutesResolved, runHookServerDone, runHookServerStart, } from '../../integrations/hooks.js'; @@ -16,7 +15,6 @@ import { createDevelopmentManifest } from '../../vite-plugin-astro-server/plugin import { createVite } from '../create-vite.js'; import type { Logger } from '../logger/core.js'; import { apply as applyPolyfill } from '../polyfill.js'; -import { injectDefaultDevRoutes } from '../routing/dev-default.js'; import { createRouteManifest } from '../routing/index.js'; import { syncInternal } from '../sync/index.js'; import { warnMissingAdapter } from './adapter-validation.js'; @@ -84,12 +82,9 @@ export async function createContainer({ .filter(Boolean) as string[]; // Create the route manifest already outside of Vite so that `runHookConfigDone` can use it to inform integrations of the build output - let manifest = await createRouteManifest({ settings, fsMod: fs }, logger, { dev: true }); + const manifest = await createRouteManifest({ settings, fsMod: fs }, logger, { dev: true }); const devSSRManifest = createDevelopmentManifest(settings); - manifest = injectDefaultDevRoutes(settings, devSSRManifest, manifest); - await runHookRoutesResolved({ settings, logger, routes: manifest.routes }); - await runHookConfigDone({ settings, logger, command: 'dev' }); warnMissingAdapter(logger, settings); diff --git a/packages/astro/src/core/routing/default.ts b/packages/astro/src/core/routing/default.ts index 8bcd473d00..d255b13275 100644 --- a/packages/astro/src/core/routing/default.ts +++ b/packages/astro/src/core/routing/default.ts @@ -1,23 +1,12 @@ -import type { ComponentInstance, ManifestData } from '../../types/astro.js'; +import type { ComponentInstance } from '../../types/astro.js'; import type { SSRManifest } from '../app/types.js'; import { DEFAULT_404_COMPONENT } from '../constants.js'; import { SERVER_ISLAND_COMPONENT, SERVER_ISLAND_ROUTE, createEndpoint as createServerIslandEndpoint, - ensureServerIslandRoute, } from '../server-islands/endpoint.js'; -import { - DEFAULT_404_ROUTE, - default404Instance, - ensure404Route, -} from './astro-designed-error-pages.js'; - -export function injectDefaultRoutes(ssrManifest: SSRManifest, routeManifest: ManifestData) { - ensure404Route(routeManifest); - ensureServerIslandRoute(ssrManifest, routeManifest); - return routeManifest; -} +import { DEFAULT_404_ROUTE, default404Instance } from './astro-designed-error-pages.js'; type DefaultRouteParams = { instance: ComponentInstance; @@ -26,6 +15,8 @@ type DefaultRouteParams = { component: string; }; +export const DEFAULT_COMPONENTS = [DEFAULT_404_COMPONENT, SERVER_ISLAND_COMPONENT]; + export function createDefaultRoutes(manifest: SSRManifest): DefaultRouteParams[] { const root = new URL(manifest.hrefRoot); return [ diff --git a/packages/astro/src/core/routing/dev-default.ts b/packages/astro/src/core/routing/dev-default.ts deleted file mode 100644 index ac2ea1d3b0..0000000000 --- a/packages/astro/src/core/routing/dev-default.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { ensureImageEndpointRoute } from '../../assets/endpoint/config.js'; -import type { AstroSettings, ManifestData } from '../../types/astro.js'; -import type { SSRManifest } from '../app/types.js'; -import { injectDefaultRoutes } from './default.js'; - -export function injectDefaultDevRoutes( - settings: AstroSettings, - ssrManifest: SSRManifest, - routeManifest: ManifestData, -) { - ensureImageEndpointRoute(settings, routeManifest, 'dev'); - injectDefaultRoutes(ssrManifest, routeManifest); - return routeManifest; -} diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 769893b87b..cfb91aacf8 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -7,6 +7,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { bold } from 'kleur/colors'; import pLimit from 'p-limit'; +import { injectImageEndpoint } from '../../../assets/endpoint/config.js'; import { toRoutingStrategy } from '../../../i18n/utils.js'; import { runHookRoutesResolved } from '../../../integrations/hooks.js'; import { getPrerenderDefault } from '../../../prerender/utils.js'; @@ -16,7 +17,9 @@ import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js'; import { MissingIndexForInternationalization } from '../../errors/errors-data.js'; import { AstroError } from '../../errors/index.js'; import { removeLeadingForwardSlash, slash } from '../../path.js'; +import { injectServerIslandRoute } from '../../server-islands/endpoint.js'; import { resolvePages } from '../../util.js'; +import { ensure404Route } from '../astro-designed-error-pages.js'; import { routeComparator } from '../priority.js'; import { getRouteGenerator } from './generator.js'; import { getPattern } from './pattern.js'; @@ -757,9 +760,22 @@ export async function createRouteManifest( } } - if (!dev) { - await runHookRoutesResolved({ routes, settings, logger }); + if (dev) { + // In SSR, a 404 route is injected in the App directly for some special handling, + // it must not appear in the manifest + ensure404Route({ routes }); } + if (dev || settings.buildOutput === 'server') { + injectImageEndpoint(settings, { routes }, dev ? 'dev' : 'build'); + // Ideally we would only inject the server islands route if server islands are used in the project. + // Unforunately, there is a "circular dependency": to know if server islands are used, we need to run + // the build but the build relies on the routes manifest. + // This situation also means we cannot update the buildOutput based on wether or not server islands + // are used in the project. If server islands are detected after the build but the buildOutput is + // static, we fail the build. + injectServerIslandRoute(settings.config, { routes }); + } + await runHookRoutesResolved({ routes, settings, logger }); return { routes, diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index ca24b54af4..5afdde651b 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -37,11 +37,7 @@ export function getServerIslandRouteData(config: ConfigFields) { return route; } -export function ensureServerIslandRoute(config: ConfigFields, routeManifest: ManifestData) { - if (routeManifest.routes.some((route) => route.route === '/_server-islands/[name]')) { - return; - } - +export function injectServerIslandRoute(config: ConfigFields, routeManifest: ManifestData) { routeManifest.routes.unshift(getServerIslandRouteData(config)); } diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index b706f967d3..7e4a7169d5 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -13,7 +13,6 @@ import { patchOverlay } from '../core/errors/overlay.js'; import type { Logger } from '../core/logger/core.js'; import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; import { createViteLoader } from '../core/module-loader/index.js'; -import { injectDefaultDevRoutes } from '../core/routing/dev-default.js'; import { createRouteManifest } from '../core/routing/index.js'; import { getRoutePrerenderOption } from '../core/routing/manifest/prerender.js'; import { toFallbackType, toRoutingStrategy } from '../i18n/utils.js'; @@ -74,15 +73,11 @@ export default function createVitePluginAstroServer({ try { const content = await fsMod.promises.readFile(routePath, 'utf-8'); await getRoutePrerenderOption(content, route, settings, logger); + await runHookRoutesResolved({ routes: routeManifest.routes, settings, logger }); } catch (_) {} } else { - routeManifest = injectDefaultDevRoutes( - settings, - devSSRManifest, - await createRouteManifest({ settings, fsMod }, logger, { dev: true }), - ); + routeManifest = await createRouteManifest({ settings, fsMod }, logger, { dev: true }); } - await runHookRoutesResolved({ routes: routeManifest.routes, settings, logger }); warnMissingAdapter(logger, settings); pipeline.manifest.checkOrigin = diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js index d4f9f25149..a981b8719c 100644 --- a/packages/astro/test/units/routing/manifest.test.js +++ b/packages/astro/test/units/routing/manifest.test.js @@ -260,6 +260,14 @@ describe('routing - createRouteManifest', () => { }); assert.deepEqual(getManifestRoutes(manifest), [ + { + route: '/_server-islands/[name]', + type: 'page', + }, + { + route: '/_image', + type: 'endpoint', + }, { route: '/blog/[...slug]', type: 'page', @@ -306,6 +314,14 @@ describe('routing - createRouteManifest', () => { }); assert.deepEqual(getManifestRoutes(manifest), [ + { + route: '/_server-islands/[name]', + type: 'page', + }, + { + route: '/_image', + type: 'endpoint', + }, { route: '/blog/about', type: 'redirect', @@ -441,6 +457,8 @@ describe('routing - createRouteManifest', () => { }); assert.deepEqual(getManifestRoutes(manifest), [ + { type: 'page', route: '/_server-islands/[name]' }, + { type: 'endpoint', route: '/_image' }, { type: 'endpoint', route: '/blog/a-[b].233' }, { type: 'redirect', route: '/posts/a-[b].233' }, { type: 'page', route: '/[c]-d' },