Skip to content

Commit

Permalink
feat: address reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
florian-lefebvre committed Dec 11, 2024
1 parent 5833c48 commit 5dec5b8
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 12 deletions.
3 changes: 3 additions & 0 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ 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;
Expand Down
16 changes: 15 additions & 1 deletion packages/astro/src/core/routing/default.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
import type { ComponentInstance } from '../../types/astro.js';
import { injectImageEndpoint } from '../../assets/endpoint/config.js';
import type { AstroSettings, ComponentInstance, ManifestData } 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,
injectServerIslandRoute,
} from '../server-islands/endpoint.js';
import { DEFAULT_404_ROUTE, default404Instance } from './astro-designed-error-pages.js';

export function injectDefaultRoutes(
settings: AstroSettings,
routeManifest: ManifestData,
dev: boolean,
) {
injectImageEndpoint(settings, routeManifest, dev ? 'dev' : 'build');
// During the build build, we can't only inject this route when server islands are used because:
// - To know if we use serve islands, we need to build
// - The build runs before we can inject the server island route
injectServerIslandRoute(settings.config, routeManifest);
}

type DefaultRouteParams = {
instance: ComponentInstance;
matchesComponent(filePath: URL): boolean;
Expand Down
16 changes: 5 additions & 11 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,8 @@ import { routeComparator } from '../priority.js';
import { getRouteGenerator } from './generator.js';
import { getPattern } from './pattern.js';
import { getRoutePrerenderOption } from './prerender.js';
import { injectImageEndpoint } from '../../../assets/endpoint/config.js';
import { ensure404Route } from '../astro-designed-error-pages.js';
import { injectServerIslandRoute } from '../../server-islands/endpoint.js';
import { injectDefaultRoutes } from '../default.js';
const require = createRequire(import.meta.url);

interface Item {
Expand Down Expand Up @@ -736,17 +735,12 @@ export async function createRouteManifest(
}

if (dev) {
ensure404Route({ routes });
injectImageEndpoint(settings, { routes }, 'dev');
injectServerIslandRoute(settings.config, { routes });
} else if (settings.buildOutput === 'server') {
injectImageEndpoint(settings, { routes }, 'build');
// We can't only inject this route when server islands are used because:
// - To know if we use serve islands, we need to build
// - The build runs before we can inject the server island route
injectServerIslandRoute(settings.config, { routes });
// 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') {
injectDefaultRoutes(settings, { routes }, dev);
}
await runHookRoutesResolved({ routes, settings, logger });

Expand Down

0 comments on commit 5dec5b8

Please sign in to comment.