feat(node): add trailingSlash support (#9080)

* feat(node): add trailing slash support

* add changeset

* test(node): add base route test in trailing-slash.js

detected an infinite loop in base path when trailingSlash: never

* fix(node): avoid infinite redirect when trailingSlash: never

* address test failures after rebase pt.1

* address test failures after rebase pt.2

---------

Co-authored-by: lilnasy <69170106+lilnasy@users.noreply.github.com>

.changeset/calm-jobs-pay.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/node': minor
+---
+
+Add trailingSlash support to NodeJS adapter

packages/integrations/node/src/serve-static.ts

@@ -1,5 +1,6 @@
 import path from 'node:path';
 import url from 'node:url';
+import fs from 'node:fs';
 import send from 'send';
 import type { IncomingMessage, ServerResponse } from 'node:http';
 import type { Options } from './types.js';
@@ -18,8 +19,47 @@ export function createStaticHandler(app: NodeApp, options: Options) {
 	 */
 	return (req: IncomingMessage, res: ServerResponse, ssr: () => unknown) => {
 		if (req.url) {
-			let pathname = app.removeBase(req.url);
-			pathname = decodeURI(new URL(pathname, 'http://host').pathname);
+			const [urlPath, urlQuery] = req.url.split('?');
+			const filePath = path.join(client, app.removeBase(urlPath));
+
+			let pathname: string;
+			let isDirectory = false;
+			try {
+				isDirectory = fs.lstatSync(filePath).isDirectory();
+			} catch {}
+
+			const { trailingSlash = 'ignore' } = options;
+
+			const hasSlash = urlPath.endsWith('/');
+			switch (trailingSlash) {
+				case "never":
+					if (isDirectory && (urlPath != '/') && hasSlash) {
+						pathname = urlPath.slice(0, -1) + (urlQuery ? "?" + urlQuery : "");
+						res.statusCode = 301;
+						res.setHeader('Location', pathname);
+						return res.end();
+					} else pathname = urlPath;
+					// intentionally fall through
+				case "ignore":
+					{
+						if (isDirectory && !hasSlash) {
+							pathname = urlPath + "/index.html";
+						} else
+							pathname = urlPath;
+					}
+					break;
+				case "always":
+					if (!hasSlash) {
+						pathname = urlPath + '/' +(urlQuery ? "?" + urlQuery : "");
+						res.statusCode = 301;
+						res.setHeader('Location', pathname);
+						return res.end();
+					} else
+						pathname = urlPath;
+				break;
+			}
+			// app.removeBase sometimes returns a path without a leading slash
+			pathname = prependForwardSlash(app.removeBase(pathname));
 
 			const stream = send(req, pathname, {
 				root: client,
@@ -47,20 +87,6 @@ export function createStaticHandler(app: NodeApp, options: Options) {
 					_res.setHeader('Cache-Control', 'public, max-age=31536000, immutable');
 				}
 			});
-			stream.on('directory', () => {
-				// On directory find, redirect to the trailing slash
-				let location: string;
-				if (req.url!.includes('?')) {
-					const [url1 = '', search] = req.url!.split('?');
-					location = `${url1}/?${search}`;
-				} else {
-					location = appendForwardSlash(req.url!);
-				}
-
-				res.statusCode = 301;
-				res.setHeader('Location', location);
-				res.end(location);
-			});
 			stream.on('file', () => {
 				forwardError = true;
 			});
@@ -81,6 +107,10 @@ function resolveClientDir(options: Options) {
 	return client;
 }
 
+function prependForwardSlash(pth: string) {
+	return pth.startsWith('/') ? pth : '/' + pth;
+}
+
 function appendForwardSlash(pth: string) {
 	return pth.endsWith('/') ? pth : pth + '/';
 }

packages/integrations/node/src/server.ts

@@ -8,6 +8,7 @@ import type { Options } from './types.js';
 applyPolyfills();
 export function createExports(manifest: SSRManifest, options: Options) {
 	const app = new NodeApp(manifest);
+	options.trailingSlash = manifest.trailingSlash;
 	return {
 		options: options,
 		handler:

packages/integrations/node/src/types.ts

@@ -1,5 +1,6 @@
 import type { NodeApp } from 'astro/app/node';
 import type { IncomingMessage, ServerResponse } from 'node:http';
+import type { SSRManifest } from 'astro';
 
 export interface UserOptions {
 	/**
@@ -17,6 +18,7 @@ export interface Options extends UserOptions {
 	server: string;
 	client: string;
 	assets: string;
+	trailingSlash?: SSRManifest['trailingSlash'];
 }
 
 export interface CreateServerOptions {

packages/integrations/node/test/fixtures/trailing-slash/astro.config.mjs

@@ -0,0 +1,8 @@
+import node from '@astrojs/node'
+
+export default {
+    base: '/some-base',
+    output: 'hybrid',
+    trailingSlash: 'never',
+    adapter: node({ mode: 'standalone' })
+};

packages/integrations/node/test/fixtures/trailing-slash/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@test/node-trailingslash",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/node": "workspace:*"
+  }
+}

packages/integrations/node/test/fixtures/trailing-slash/src/pages/index.astro

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

packages/integrations/node/test/fixtures/trailing-slash/src/pages/one.astro

@@ -0,0 +1,11 @@
+---
+export const prerender = true;
+---
+<html>
+	<head>
+		<title>One</title>
+	</head>
+	<body>
+		<h1>One</h1>
+	</body>
+</html>

packages/integrations/node/test/prerender.test.js

@@ -74,12 +74,14 @@ describe('Prerendering', () => {
 			expect($('h1').text()).to.equal('Two');
 		});
 
-		it('Omitting the trailing slash results in a redirect that includes the base', async () => {
+		it('Can render prerendered route without trailing slash', async () => {
 			const res = await fetch(`http://${server.host}:${server.port}/some-base/two`, {
 				redirect: 'manual',
 			});
-			expect(res.status).to.equal(301);
-			expect(res.headers.get('location')).to.equal('/some-base/two/');
+			const html = await res.text();
+			const $ = cheerio.load(html);
+			expect(res.status).to.equal(200);
+			expect($('h1').text()).to.equal('Two');
 		});
 	});
 
@@ -241,12 +243,14 @@ describe('Hybrid rendering', () => {
 			expect($('h1').text()).to.equal('One');
 		});
 
-		it('Omitting the trailing slash results in a redirect that includes the base', async () => {
+		it('Can render prerendered route without trailing slash', async () => {
 			const res = await fetch(`http://${server.host}:${server.port}/some-base/one`, {
 				redirect: 'manual',
 			});
-			expect(res.status).to.equal(301);
-			expect(res.headers.get('location')).to.equal('/some-base/one/');
+			const html = await res.text();
+			const $ = cheerio.load(html);
+			expect(res.status).to.equal(200);
+			expect($('h1').text()).to.equal('One');
 		});
 	});
 

packages/integrations/node/test/trailing-slash.js

@@ -0,0 +1,405 @@
+import nodejs from '../dist/index.js';
+import { loadFixture } from './test-utils.js';
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+
+/**
+ * @typedef {import('../../../astro/test/test-utils').Fixture} Fixture
+ */
+
+async function load() {
+	const mod = await import(`./fixtures/trailing-slash/dist/server/entry.mjs?dropcache=${Date.now()}`);
+	return mod;
+}
+
+describe('Trailing slash', () => {
+	/** @type {import('./test-utils').Fixture} */
+	let fixture;
+	let server;
+	describe('Always', async () => {
+		describe('With base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					base: '/some-base',
+					output: 'hybrid',
+					trailingSlash: 'always',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with redirect', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/some-base/one/');
+			});
+
+			it('Can render prerendered route with redirect and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one?foo=bar`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/some-base/one/?foo=bar');
+			});
+
+			it('Can render prerendered route with query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one/?foo=bar`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+		describe('Without base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					output: 'hybrid',
+					trailingSlash: 'always',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with redirect', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/one/');
+			});
+
+			it('Can render prerendered route with redirect and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/one/?foo=bar');
+			});
+
+			it('Can render prerendered route with query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one/?foo=bar`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+	});
+	describe('Never', async () => {
+		describe('With base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					base: '/some-base',
+					output: 'hybrid',
+					trailingSlash: 'never',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with redirect', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one/`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/some-base/one');
+			});
+
+			it('Can render prerendered route with redirect and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one/?foo=bar`, {
+					redirect : 'manual'
+				});
+
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/some-base/one?foo=bar');
+			});
+
+			it('Can render prerendered route with query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one?foo=bar`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+		describe('Without base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					output: 'hybrid',
+					trailingSlash: 'never',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with redirect', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one/`, {
+					redirect : 'manual'
+				});
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/one');
+			});
+
+			it('Can render prerendered route with redirect and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one/?foo=bar`, {
+					redirect : 'manual'
+				});
+
+				expect(res.status).to.equal(301);
+				expect(res.headers.get('location')).to.equal('/one?foo=bar');
+			});
+
+			it('Can render prerendered route and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+	});
+	describe('Ignore', async () => {
+		describe('With base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					base: '/some-base',
+					output: 'hybrid',
+					trailingSlash: 'ignore',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with slash', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one/`, {
+					redirect : 'manual'
+				});
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route without slash', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one`, {
+					redirect : 'manual'
+				});
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route with slash and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one/?foo=bar`, {
+					redirect : 'manual'
+				});
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route without slash and with query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/some-base/one?foo=bar`, {
+					redirect : 'manual'
+				});
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+		describe('Without base', async () => {
+			before(async () => {
+				process.env.ASTRO_NODE_AUTOSTART = 'disabled';
+				process.env.PRERENDER = true;
+
+				fixture = await loadFixture({
+					root: './fixtures/trailing-slash/',
+					output: 'hybrid',
+					trailingSlash: 'ignore',
+					adapter: nodejs({ mode: 'standalone' }),
+				});
+				await fixture.build();
+				const { startServer } = await load();
+				let res = startServer();
+				server = res.server;
+			});
+
+			after(async () => {
+				await server.stop();
+				await fixture.clean();
+				delete process.env.PRERENDER;
+			});
+
+			it('Can render prerendered base route', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('Index');
+			});
+
+			it('Can render prerendered route with slash', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one/`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route without slash', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route with slash and query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one/?foo=bar`, {
+					redirect : 'manual'
+				});
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+
+			it('Can render prerendered route without slash and with query params', async () => {
+				const res = await fetch(`http://${server.host}:${server.port}/one?foo=bar`);
+				const html = await res.text();
+				const $ = cheerio.load(html);
+
+				expect(res.status).to.equal(200);
+				expect($('h1').text()).to.equal('One');
+			});
+		});
+	});
+});
+

pnpm-lock.yaml

@@ -4456,6 +4456,15 @@ importers:
         specifier: workspace:*
         version: link:../../../../../astro
 
+  packages/integrations/node/test/fixtures/trailing-slash:
+    dependencies:
+      '@astrojs/node':
+        specifier: workspace:*
+        version: link:../../..
+      astro:
+        specifier: workspace:*
+        version: link:../../../../../astro
+
   packages/integrations/node/test/fixtures/url-protocol:
     dependencies:
       '@astrojs/node':