Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-03-31 23:31:30 -05:00
Code Issues Activity

fix(rewrite): allow to rewrite 404 and take base into consideration (#11272)

Browse source 
* fix(rewrite): allow to rewrite 404

* add changesets

* rebase

* apply suggestion

* Update .changeset/honest-shirts-trade.md

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>

---------

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
This commit is contained in:
Emanuele Stoppa 2024-06-19 14:25:27 +01:00 • committed by GitHub
parent bc3c3296bf
commit ea987d7da5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 214 additions and 196 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/five-owls-check.md Normal file
View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Fixes a case where rewriting `/` would cause an issue, when `trailingSlash` was set to `"never"`.

5
.changeset/honest-shirts-trade.md Normal file
View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Reverts a logic where it wasn't possible to rewrite `/404` in static mode. It's **now possible** again

49
packages/astro/src/container/pipeline.ts
View file

@ -7,13 +7,14 @@ import type {
} from '../@types/astro.js';
import { type HeadElements, Pipeline } from '../core/base-pipeline.js';
import type { SinglePageBuiltModule } from '../core/build/types.js';
import { InvalidRewrite404, RouteNotFound } from '../core/errors/errors-data.js';
import { RouteNotFound } from '../core/errors/errors-data.js';
import { AstroError } from '../core/errors/index.js';
import {
createModuleScriptElement,
createStylesheetElementSet,
} from '../core/render/ssr-element.js';
import { DEFAULT_404_ROUTE, default404Page } from '../core/routing/astro-designed-error-pages.js';
import { DEFAULT_404_ROUTE } from '../core/routing/astro-designed-error-pages.js';
import {findRouteToRewrite} from "../core/routing/rewrite.js";
export class ContainerPipeline extends Pipeline {
/**
@ -74,31 +75,17 @@ export class ContainerPipeline extends Pipeline {
payload: RewritePayload,
request: Request
): Promise<[RouteData, ComponentInstance, URL]> {
let foundRoute: RouteData | undefined;
// options.manifest is the actual type that contains the information
let finalUrl: URL | undefined = undefined;
for (const route of this.manifest.routes) {
if (payload instanceof URL) {
finalUrl = payload;
} else if (payload instanceof Request) {
finalUrl = new URL(payload.url);
} else {
finalUrl = new URL(payload, new URL(request.url).origin);
}
if (route.routeData.pattern.test(decodeURI(finalUrl.pathname))) {
foundRoute = route.routeData;
break;
} else if (finalUrl.pathname === '/404') {
foundRoute = DEFAULT_404_ROUTE;
break;
}
}
if (foundRoute && finalUrl) {
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
} else {
throw new AstroError(RouteNotFound);
}
const [foundRoute, finalUrl] = findRouteToRewrite({
payload,
request,
routes: this.manifest?.routes.map((r) => r.routeData),
trailingSlash: this.manifest.trailingSlash,
buildFormat: this.manifest.buildFormat,
base: this.manifest.base,
});
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
insertRoute(route: RouteData, componentInstance: ComponentInstance): void {
@ -114,12 +101,4 @@ export class ContainerPipeline extends Pipeline {
// At the moment it's not used by the container via any public API
// @ts-expect-error It needs to be implemented.
async getComponentByRoute(_routeData: RouteData): Promise<ComponentInstance> {}
rewriteKnownRoute(pathname: string, _sourceRoute: RouteData): ComponentInstance {
if (pathname === '/404') {
return { default: default404Page } as any as ComponentInstance;
}
throw new AstroError(InvalidRewrite404);
}
}

55
packages/astro/src/core/app/pipeline.ts
View file

@ -14,6 +14,7 @@ import { AstroError } from '../errors/index.js';
import { RedirectSinglePageBuiltModule } from '../redirects/component.js';
import { createModuleScriptElement, createStylesheetElementSet } from '../render/ssr-element.js';
import { DEFAULT_404_ROUTE } from '../routing/astro-designed-error-pages.js';
import { findRouteToRewrite } from '../routing/rewrite.js';
export class AppPipeline extends Pipeline {
#manifestData: ManifestData | undefined;
@ -86,42 +87,19 @@ export class AppPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
request: Request,
sourceRoute: RouteData
_sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance, URL]> {
let foundRoute;
let finalUrl: URL | undefined = undefined;
for (const route of this.#manifestData!.routes) {
if (payload instanceof URL) {
finalUrl = payload;
} else if (payload instanceof Request) {
finalUrl = new URL(payload.url);
} else {
finalUrl = new URL(payload, new URL(request.url).origin);
}
if (route.pattern.test(decodeURI(finalUrl.pathname))) {
foundRoute = route;
break;
} else if (finalUrl.pathname === '/404') {
foundRoute = DEFAULT_404_ROUTE;
break;
}
}
if (foundRoute && finalUrl) {
if (foundRoute.pathname === '/404') {
const componentInstance = this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
return [foundRoute, componentInstance, finalUrl];
} else {
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
}
throw new AstroError({
...RewriteEncounteredAnError,
message: RewriteEncounteredAnError.message(payload.toString()),
const [foundRoute, finalUrl] = findRouteToRewrite({
payload,
request,
routes: this.manifest?.routes.map((r) => r.routeData),
trailingSlash: this.manifest.trailingSlash,
buildFormat: this.manifest.buildFormat,
base: this.manifest.base,
});
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
async getModuleForRoute(route: RouteData): Promise<SinglePageBuiltModule> {
@ -151,13 +129,4 @@ export class AppPipeline extends Pipeline {
);
}
}
// We don't need to check the source route, we already are in SSR
rewriteKnownRoute(pathname: string, _sourceRoute: RouteData): ComponentInstance {
if (pathname === '/404') {
return { default: () => new Response(null, { status: 404 }) } as ComponentInstance;
} else {
return { default: () => new Response(null, { status: 500 }) } as ComponentInstance;
}
}
}

10
packages/astro/src/core/base-pipeline.ts
View file

@ -95,16 +95,6 @@ export abstract class Pipeline {
* @param routeData
*/
abstract getComponentByRoute(routeData: RouteData): Promise<ComponentInstance>;
/**
* Attempts to execute a rewrite of a known Astro route:
* - /404 -> src/pages/404.astro -> template
* - /500 -> src/pages/500.astro
*
* @param pathname The pathname where the user wants to rewrite e.g. "/404"
* @param sourceRoute The original route where the first request started. This is needed in case a pipeline wants to check if the current route is pre-rendered or not
*/
abstract rewriteKnownRoute(pathname: string, sourceRoute: RouteData): ComponentInstance;
}
// eslint-disable-next-line @typescript-eslint/no-empty-interface

60
packages/astro/src/core/build/pipeline.ts
View file

@ -8,8 +8,6 @@ import type {
import { getOutputDirectory } from '../../prerender/utils.js';
import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
import type { SSRManifest } from '../app/types.js';
import { InvalidRewrite404, RewriteEncounteredAnError } from '../errors/errors-data.js';
import { AstroError } from '../errors/index.js';
import { routeIsFallback, routeIsRedirect } from '../redirects/helpers.js';
import { RedirectSinglePageBuiltModule } from '../redirects/index.js';
import { Pipeline } from '../render/index.js';
@ -18,7 +16,6 @@ import {
createModuleScriptsSet,
createStylesheetElementSet,
} from '../render/ssr-element.js';
import { DEFAULT_404_ROUTE } from '../routing/astro-designed-error-pages.js';
import { isServerLikeOutput } from '../util.js';
import { getOutDirWithinCwd } from './common.js';
import { type BuildInternals, cssOrder, getPageData, mergeInlineCss } from './internal.js';
@ -27,6 +24,9 @@ import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js';
import { getPagesFromVirtualModulePageName, getVirtualModulePageName } from './plugins/util.js';
import type { PageBuildData, SinglePageBuiltModule, StaticBuildOptions } from './types.js';
import { i18nHasFallback } from './util.js';
import { findRouteToRewrite } from '../routing/rewrite.js';
import {DEFAULT_404_COMPONENT} from "../constants.js";
import {default404Page} from "../routing/astro-designed-error-pages.js";
/**
* The build pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files.
@ -269,6 +269,8 @@ export class BuildPipeline extends Pipeline {
// SAFETY: checked before
const entry = this.#componentsInterner.get(routeData)!;
return await entry.page();
} else if (routeData.component === DEFAULT_404_COMPONENT) {
return { default: default404Page }
} else {
// SAFETY: the pipeline calls `retrieveRoutesToGenerate`, which is in charge to fill the cache.
const filePath = this.#routesByFilePath.get(routeData)!;
@ -280,42 +282,19 @@ export class BuildPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
request: Request,
sourceRoute: RouteData
_sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance, URL]> {
let foundRoute: RouteData | undefined;
// options.manifest is the actual type that contains the information
let finalUrl: URL | undefined = undefined;
for (const route of this.options.manifest.routes) {
if (payload instanceof URL) {
finalUrl = payload;
} else if (payload instanceof Request) {
finalUrl = new URL(payload.url);
} else {
finalUrl = new URL(payload, new URL(request.url).origin);
}
const [foundRoute, finalUrl] = findRouteToRewrite({
payload,
request,
routes: this.options.manifest.routes,
trailingSlash: this.config.trailingSlash,
buildFormat: this.config.build.format,
base: this.config.base
});
if (route.pattern.test(decodeURI(finalUrl.pathname))) {
foundRoute = route;
break;
} else if (finalUrl.pathname === '/404') {
foundRoute = DEFAULT_404_ROUTE;
break;
}
}
if (foundRoute && finalUrl) {
if (foundRoute.pathname === '/404') {
const componentInstance = await this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
return [foundRoute, componentInstance, finalUrl];
} else {
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
} else {
throw new AstroError({
...RewriteEncounteredAnError,
message: RewriteEncounteredAnError.message(payload.toString()),
});
}
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
async retrieveSsrEntry(route: RouteData, filePath: string): Promise<SinglePageBuiltModule> {
@ -375,13 +354,6 @@ export class BuildPipeline extends Pipeline {
return RedirectSinglePageBuiltModule;
}
rewriteKnownRoute(_pathname: string, sourceRoute: RouteData): ComponentInstance {
if (!isServerLikeOutput(this.config) || sourceRoute.prerender) {
throw new AstroError(InvalidRewrite404);
}
throw new Error(`Unreachable, in SSG this route shouldn't be generated`);
}
}
function createEntryURL(filePath: string, outFolder: URL) {

12
packages/astro/src/core/errors/errors-data.ts
View file

@ -1155,18 +1155,6 @@ export const RewriteEncounteredAnError = {
}.`,
} satisfies ErrorData;
/**
* @docs
* @description
*
* The user tried to rewrite a 404 page inside a static page.
*/
export const InvalidRewrite404 = {
name: 'InvalidRewrite404',
title: "You attempted to rewrite a 404 inside a static page, and this isn't allowed.",
message: 'Rewriting a 404 is only allowed inside on-demand pages.',
} satisfies ErrorData;
/**
* @docs
* @description

55
packages/astro/src/core/routing/rewrite.ts Normal file
View file

@ -0,0 +1,55 @@
import type { AstroConfig, RewritePayload, RouteData } from '../../@types/astro.js';
import { shouldAppendForwardSlash } from '../build/util.js';
import { DEFAULT_404_ROUTE } from './astro-designed-error-pages.js';
import { appendForwardSlash, removeTrailingForwardSlash } from '../path.js';
export type FindRouteToRewrite = {
payload: RewritePayload;
routes: RouteData[];
request: Request;
trailingSlash: AstroConfig['trailingSlash'];
buildFormat: AstroConfig['build']['format'];
base: AstroConfig['base'];
};
export function findRouteToRewrite({
payload,
routes,
request,
trailingSlash,
buildFormat,
base,
}: FindRouteToRewrite): [RouteData, URL] {
let finalUrl: URL | undefined = undefined;
if (payload instanceof URL) {
finalUrl = payload;
} else if (payload instanceof Request) {
finalUrl = new URL(payload.url);
} else {
finalUrl = new URL(payload, new URL(request.url).origin);
}
let foundRoute;
for (const route of routes) {
const pathname = shouldAppendForwardSlash(trailingSlash, buildFormat)
? appendForwardSlash(finalUrl.pathname)
: base !== '/'
? removeTrailingForwardSlash(finalUrl.pathname)
: finalUrl.pathname;
if (route.pattern.test(decodeURI(pathname))) {
foundRoute = route;
break;
}
}
if (foundRoute) {
return [foundRoute, finalUrl];
} else {
const custom404 = routes.find((route) => route.route === '/404');
if (custom404) {
return [custom404, finalUrl];
} else {
return [DEFAULT_404_ROUTE, finalUrl];
}
}
}

61
packages/astro/src/vite-plugin-astro-server/pipeline.ts
View file

@ -14,7 +14,7 @@ import { getInfoOutput } from '../cli/info/index.js';
import { type HeadElements } from '../core/base-pipeline.js';
import { ASTRO_VERSION, DEFAULT_404_COMPONENT } from '../core/constants.js';
import { enhanceViteSSRError } from '../core/errors/dev/index.js';
import { InvalidRewrite404, RewriteEncounteredAnError } from '../core/errors/errors-data.js';
import { RewriteEncounteredAnError } from '../core/errors/errors-data.js';
import { AggregateError, AstroError, CSSError, MarkdownError } from '../core/errors/index.js';
import type { Logger } from '../core/logger/core.js';
import type { ModuleLoader } from '../core/module-loader/index.js';
@ -26,6 +26,9 @@ import { getStylesForURL } from './css.js';
import { getComponentMetadata } from './metadata.js';
import { createResolve } from './resolve.js';
import { getScriptsForURL } from './scripts.js';
import { shouldAppendForwardSlash } from '../core/build/util.js';
import { prependForwardSlash, removeTrailingForwardSlash } from '../core/path.js';
import { findRouteToRewrite } from '../core/routing/rewrite.js';
export class DevPipeline extends Pipeline {
// renderers are loaded on every request,
@ -194,59 +197,25 @@ export class DevPipeline extends Pipeline {
async tryRewrite(
payload: RewritePayload,
request: Request,
sourceRoute: RouteData
_sourceRoute: RouteData
): Promise<[RouteData, ComponentInstance, URL]> {
let foundRoute;
if (!this.manifestData) {
throw new Error('Missing manifest data. This is an internal error, please file an issue.');
}
const [foundRoute, finalUrl] = findRouteToRewrite({
payload,
request,
routes: this.manifestData?.routes,
trailingSlash: this.config.trailingSlash,
buildFormat: this.config.build.format,
base: this.config.base
});
let finalUrl: URL | undefined = undefined;
for (const route of this.manifestData.routes) {
if (payload instanceof URL) {
finalUrl = payload;
} else if (payload instanceof Request) {
finalUrl = new URL(payload.url);
} else {
finalUrl = new URL(payload, new URL(request.url).origin);
}
if (route.pattern.test(decodeURI(finalUrl.pathname))) {
foundRoute = route;
break;
} else if (finalUrl.pathname === '/404') {
foundRoute = DEFAULT_404_ROUTE;
break;
}
}
if (foundRoute && finalUrl) {
if (foundRoute.pathname === '/404') {
const componentInstance = this.rewriteKnownRoute(foundRoute.pathname, sourceRoute);
return [foundRoute, componentInstance, finalUrl];
} else {
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
} else {
throw new AstroError({
...RewriteEncounteredAnError,
message: RewriteEncounteredAnError.message(payload.toString()),
});
}
const componentInstance = await this.getComponentByRoute(foundRoute);
return [foundRoute, componentInstance, finalUrl];
}
setManifestData(manifestData: ManifestData) {
this.manifestData = manifestData;
}
rewriteKnownRoute(route: string, sourceRoute: RouteData): ComponentInstance {
if (isServerLikeOutput(this.config) && sourceRoute.prerender) {
if (route === '/404') {
return { default: default404Page } as any as ComponentInstance;
}
}
throw new AstroError(InvalidRewrite404);
}
}

11
packages/astro/test/fixtures/rewrite-trailing-slash-never/astro.config.mjs vendored Normal file
View file

@ -0,0 +1,11 @@
import { defineConfig } from 'astro/config';
// https://astro.build/config
export default defineConfig({
trailingSlash: "never",
experimental: {
rewriting: true
},
base: "base",
site: "https://example.com"
});

8
packages/astro/test/fixtures/rewrite-trailing-slash-never/package.json vendored Normal file
View file

@ -0,0 +1,8 @@
{
"name": "@test/rewrite-trailing-slash-never",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}

3
packages/astro/test/fixtures/rewrite-trailing-slash-never/src/pages/foo.astro vendored Normal file
View file

@ -0,0 +1,3 @@
---
return Astro.rewrite("/")
---

8
packages/astro/test/fixtures/rewrite-trailing-slash-never/src/pages/index.astro vendored Normal file
View file

@ -0,0 +1,8 @@
<html>
<head>
<title>Index</title>
</head>
<body>
<h1>Index</h1>
</body>
</html>

62
packages/astro/test/rewrite.test.js
View file

@ -62,6 +62,56 @@ describe('Dev reroute', () => {
});
});
describe('Dev rewrite, trailing slash -> never', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let devServer;
before(async () => {
fixture = await loadFixture({
root: './fixtures/rewrite-trailing-slash-never/',
});
devServer = await fixture.startDevServer();
});
after(async () => {
await devServer.stop();
});
it('should rewrite to the homepage', async () => {
const html = await fixture.fetch('/foo').then((res) => res.text());
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
});
describe('Dev rewrite, trailing slash -> never, with base', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let devServer;
before(async () => {
fixture = await loadFixture({
root: './fixtures/rewrite-trailing-slash-never/',
base: "base",
});
devServer = await fixture.startDevServer();
});
after(async () => {
await devServer.stop();
});
it('should rewrite to the homepage', async () => {
const html = await fixture.fetch('/foo').then((res) => res.text());
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
});
describe('Dev rewrite, hybrid/server', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
@ -113,21 +163,21 @@ describe('Build reroute', () => {
await fixture.build();
});
it('should render the index page when navigating /reroute ', async () => {
it('should create the index page when navigating /reroute ', async () => {
const html = await fixture.readFile('/reroute/index.html');
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
it('should render the index page when navigating /blog/hello ', async () => {
it('should create the index page when navigating /blog/hello ', async () => {
const html = await fixture.readFile('/blog/hello/index.html');
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
it('should render the index page when navigating /blog/salut ', async () => {
it('should create the index page when navigating /blog/salut ', async () => {
const html = await fixture.readFile('/blog/salut/index.html');
const $ = cheerioLoad(html);
@ -135,21 +185,21 @@ describe('Build reroute', () => {
assert.equal($('h1').text(), 'Index');
});
it('should render the index page when navigating dynamic route /dynamic/[id] ', async () => {
it('should create the index page when navigating dynamic route /dynamic/[id] ', async () => {
const html = await fixture.readFile('/dynamic/hello/index.html');
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
it('should render the index page when navigating spread route /spread/[...spread] ', async () => {
it('should create the index page when navigating spread route /spread/[...spread] ', async () => {
const html = await fixture.readFile('/spread/hello/index.html');
const $ = cheerioLoad(html);
assert.equal($('h1').text(), 'Index');
});
it('should render the 404 built-in page', async () => {
it('should create the 404 built-in page', async () => {
try {
await fixture.readFile('/spread/oops/index.html');
assert.fail('Not found');

6
pnpm-lock.yaml generated
View file

@ -3535,6 +3535,12 @@ importers:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/rewrite-trailing-slash-never:
dependencies:
astro:
specifier: workspace:*
version: link:../../..
packages/astro/test/fixtures/root-srcdir-css:
dependencies:
astro: