fix: better dev routing with base using middleware (#3942)

.changeset/dull-eagles-beg.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Use a base middleware for better base path handling in dev.

packages/astro/src/vite-plugin-astro-server/index.ts

@@ -114,38 +114,14 @@ async function handle404Response(
 	req: http.IncomingMessage,
 	res: http.ServerResponse
 ) {
-	const site = config.site ? new URL(config.base, config.site) : undefined;
-	const devRoot = site ? site.pathname : '/';
 	const pathname = decodeURI(new URL(origin + req.url).pathname);
-	let html = '';
-	if (pathname === '/' && !pathname.startsWith(devRoot)) {
-		html = subpathNotUsedTemplate(devRoot, pathname);
-	} else {
-		// HACK: redirect without the base path for assets in publicDir
-		const redirectTo =
-			req.method === 'GET' &&
-			config.base !== '/' &&
-			pathname.startsWith(config.base) &&
-			pathname.replace(config.base, '/');
 
-		if (redirectTo && redirectTo !== '/') {
-			const response = new Response(null, {
-				status: 302,
-				headers: {
-					Location: redirectTo,
-				},
-			});
-			await writeWebResponse(res, response);
-			return;
-		}
-
-		html = notFoundTemplate({
+	const html = notFoundTemplate({
 		statusCode: 404,
 		title: 'Not found',
 		tabTitle: '404: Not Found',
 		pathname,
 	});
-	}
 	writeHtmlResponse(res, 404, html);
 }
 
@@ -179,6 +155,44 @@ function log404(logging: LogOptions, pathname: string) {
 	info(logging, 'serve', msg.req({ url: pathname, statusCode: 404 }));
 }
 
+export function baseMiddleware(
+	config: AstroConfig,
+	logging: LogOptions
+): vite.Connect.NextHandleFunction {
+	const site = config.site ? new URL(config.base, config.site) : undefined;
+	const devRoot = site ? site.pathname : '/';
+
+	return function devBaseMiddleware(req, res, next) {
+		const url = req.url!;
+
+		const pathname = decodeURI(new URL(url, 'http://vitejs.dev').pathname);
+
+		if (pathname.startsWith(devRoot)) {
+			req.url = url.replace(devRoot, '/');
+			return next();
+		}
+
+		if (pathname === '/' || pathname === '/index.html') {
+			log404(logging, pathname);
+			const html = subpathNotUsedTemplate(devRoot, pathname);
+			return writeHtmlResponse(res, 404, html);
+		}
+
+		if (req.headers.accept?.includes('text/html')) {
+			log404(logging, pathname);
+			const html = notFoundTemplate({
+				statusCode: 404,
+				title: 'Not found',
+				tabTitle: '404: Not Found',
+				pathname,
+			});
+			return writeHtmlResponse(res, 404, html);
+		}
+
+		next();
+	};
+}
+
 /** The main logic to route dev server requests to pages in Astro. */
 async function handleRequest(
 	routeCache: RouteCache,
@@ -190,8 +204,6 @@ async function handleRequest(
 	res: http.ServerResponse
 ) {
 	const reqStart = performance.now();
-	const site = config.site ? new URL(config.base, config.site) : undefined;
-	const devRoot = site ? site.pathname : '/';
 	const origin = `${viteServer.config.server.https ? 'https' : 'http'}://${req.headers.host}`;
 	const buildingToSSR = isBuildingToSSR(config);
 	// Ignore `.html` extensions and `index.html` in request URLS to ensure that
@@ -199,10 +211,12 @@ async function handleRequest(
 	// build formats, and is necessary based on how the manifest tracks build targets.
 	const url = new URL(origin + req.url?.replace(/(index)?\.html$/, ''));
 	const pathname = decodeURI(url.pathname);
-	const rootRelativeUrl = pathname.substring(devRoot.length - 1);
+
+	// Add config.base back to url before passing it to SSR
+	url.pathname = config.base.substring(0, config.base.length - 1) + url.pathname;
 
 	// HACK! @astrojs/image uses query params for the injected route in `dev`
-	if (!buildingToSSR && rootRelativeUrl !== '/_image') {
+	if (!buildingToSSR && pathname !== '/_image') {
 		// Prevent user from depending on search params when not doing SSR.
 		// NOTE: Create an array copy here because deleting-while-iterating
 		// creates bugs where not all search params are removed.
@@ -236,13 +250,9 @@ async function handleRequest(
 
 	let filePath: URL | undefined;
 	try {
-		if (!pathname.startsWith(devRoot)) {
-			log404(logging, pathname);
-			return handle404Response(origin, config, req, res);
-		}
 		// Attempt to match the URL to a valid page route.
 		// If that fails, switch the response to a 404 response.
-		let route = matchRoute(rootRelativeUrl, manifest);
+		let route = matchRoute(pathname, manifest);
 		const statusCode = route ? 200 : 404;
 
 		if (!route) {
@@ -264,7 +274,7 @@ async function handleRequest(
 			mod,
 			route,
 			routeCache,
-			pathname: rootRelativeUrl,
+			pathname: pathname,
 			logging,
 			ssr: isBuildingToSSR(config),
 		});
@@ -289,7 +299,7 @@ async function handleRequest(
 					logging,
 					mode: 'development',
 					origin,
-					pathname: rootRelativeUrl,
+					pathname: pathname,
 					request,
 					route: routeCustom404,
 					routeCache,
@@ -307,7 +317,7 @@ async function handleRequest(
 			logging,
 			mode: 'development',
 			origin,
-			pathname: rootRelativeUrl,
+			pathname: pathname,
 			route,
 			routeCache,
 			viteServer,
@@ -390,6 +400,12 @@ export default function createPlugin({ config, logging }: AstroPluginOptions): v
 					route: '',
 					handle: forceTextCSSForStylesMiddleware,
 				});
+				if (config.base !== '/') {
+					viteServer.middlewares.stack.unshift({
+						route: '',
+						handle: baseMiddleware(config, logging),
+					});
+				}
 				viteServer.middlewares.use(async (req, res) => {
 					if (!req.url || !req.method) {
 						throw new Error('Incomplete request');

packages/astro/test/fixtures/public-base-404/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@test/public-base-404",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/public-base-404/src/pages/404.astro

@@ -0,0 +1,8 @@
+<html lang="en">
+<head>
+  <title>Not Found</title>
+</head>
+<body>
+  <h1>404</h1>
+</body>
+</html>

packages/astro/test/fixtures/public-base-404/src/pages/index.astro

@@ -0,0 +1,8 @@
+<html lang="en">
+<head>
+  <title>This Site</title>
+</head>
+<body>
+  <img src="/twitter.png"  />
+</body>
+</html>

packages/astro/test/public-base-404.test.js

@@ -0,0 +1,64 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('Public dev with base', () => {
+  /** @type {import('./test-utils').Fixture} */
+  let fixture;
+  /** @type {import('./test-utils').DevServer} */
+  let devServer;
+  let $;
+
+  before(async () => {
+    fixture = await loadFixture({
+      root: './fixtures/public-base-404/',
+      site: 'http://example.com/',
+      base: '/blog'
+    });
+    devServer = await fixture.startDevServer();
+  });
+
+  it('200 when loading /@vite/client', async () => {
+    const response = await fixture.fetch('/@vite/client', {
+      redirect: 'manual'
+    });
+    expect(response.status).to.equal(200);
+    const content = await response.text()
+    expect(content).to.contain('vite')
+  });
+
+  it('200 when loading /blog/twitter.png', async () => {
+    const response = await fixture.fetch('/blog/twitter.png', {
+      redirect: 'manual'
+    });
+    expect(response.status).to.equal(200);
+  });
+
+  it('custom 404 page when loading /blog/blog/', async () => {
+    const response = await fixture.fetch('/blog/blog/');
+    const html = await response.text()
+    $ = cheerio.load(html);
+    expect($('h1').text()).to.equal('404');
+  });
+
+  it('default 404 hint page when loading /', async () => {
+    const response = await fixture.fetch('/');
+    expect(response.status).to.equal(404);
+    const html = await response.text()
+    $ = cheerio.load(html);
+    expect($('a').first().text()).to.equal('/blog/');
+  });
+
+  it('default 404 page when loading /none/', async () => {
+    const response = await fixture.fetch('/none/', {
+      headers: {
+        accept: 'text/html,*/*'
+      }
+    });
+    expect(response.status).to.equal(404);
+    const html = await response.text()
+    $ = cheerio.load(html);
+    expect($('h1').text()).to.equal('404:  Not found');
+    expect($('pre').text()).to.equal('Path: /none/');
+  });
+});

pnpm-lock.yaml

@@ -1618,6 +1618,12 @@ importers:
       '@astrojs/preact': link:../../../../integrations/preact
       astro: link:../../..
 
+  packages/astro/test/fixtures/public-base-404:
+    specifiers:
+      astro: workspace:*
+    dependencies:
+      astro: link:../../..
+
   packages/astro/test/fixtures/react-component:
     specifiers:
       '@astrojs/react': workspace:*