Skip to content
This repository has been archived by the owner on Feb 10, 2025. It is now read-only.

fix: use vercel routing utils #525

Merged
merged 8 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-walls-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/vercel': patch
---

Handle trailing slashes correctly
19 changes: 15 additions & 4 deletions packages/vercel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
"url": "https://github.com/withastro/adapters.git",
"directory": "packages/vercel"
},
"keywords": ["withastro", "astro-adapter"],
"keywords": [
"withastro",
"astro-adapter"
],
"bugs": "https://github.com/withastro/adapters/issues",
"homepage": "https://docs.astro.build/en/guides/integrations-guide/vercel/",
"exports": {
Expand All @@ -25,11 +28,18 @@
},
"typesVersions": {
"*": {
"serverless": ["dist/serverless/adapter.d.ts"],
"static": ["dist/static/adapter.d.ts"]
"serverless": [
"dist/serverless/adapter.d.ts"
],
"static": [
"dist/static/adapter.d.ts"
]
}
},
"files": ["dist", "types.d.ts"],
"files": [
"dist",
"types.d.ts"
],
"scripts": {
"build": "tsc",
"test": "astro-scripts test --timeout 50000 \"test/**/!(hosted).test.js\"",
Expand All @@ -40,6 +50,7 @@
"@vercel/analytics": "^1.4.1",
"@vercel/edge": "^1.2.1",
"@vercel/nft": "^0.29.0",
"@vercel/routing-utils": "^5.0.0",
"esbuild": "^0.24.0",
"fast-glob": "^3.3.3"
},
Expand Down
52 changes: 40 additions & 12 deletions packages/vercel/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
getInjectableWebAnalyticsContent,
} from './lib/web-analytics.js';
import { generateEdgeMiddleware } from './serverless/middleware.js';
import { getTransformedRoutes, normalizeRoutes, type Route } from '@vercel/routing-utils';

const PACKAGE_NAME = '@astrojs/vercel';

Expand Down Expand Up @@ -261,16 +262,23 @@ export default function vercelAdapter({
);
}
const vercelConfigPath = new URL('vercel.json', config.root);
if (existsSync(vercelConfigPath)) {
if (
config.trailingSlash &&
config.trailingSlash !== 'ignore' &&
existsSync(vercelConfigPath)
) {
try {
const vercelConfig = JSON.parse(readFileSync(vercelConfigPath, 'utf-8'));
if (vercelConfig.trailingSlash === true && config.trailingSlash === 'always') {
logger.warn(
'\n' +
`\tYour "vercel.json" \`trailingSlash\` configuration (set to \`true\`) will conflict with your Astro \`trailinglSlash\` configuration (set to \`"always"\`).\n` +
// biome-ignore lint/style/noUnusedTemplateLiteral: <explanation>
`\tThis would cause infinite redirects under certain conditions and throw an \`ERR_TOO_MANY_REDIRECTS\` error.\n` +
`\tTo prevent this, change your Astro configuration and update \`"trailingSlash"\` to \`"ignore"\`.\n`
if (
(vercelConfig.trailingSlash === true && config.trailingSlash === 'never') ||
(vercelConfig.trailingSlash === false && config.trailingSlash === 'always')
) {
logger.error(
`
Your "vercel.json" \`trailingSlash\` configuration (set to \`${vercelConfig.trailingSlash}\`) will conflict with your Astro \`trailingSlash\` configuration (set to \`${JSON.stringify(config.trailingSlash)}\`).
This would cause infinite redirects or duplicate content issues.
Please remove the \`trailingSlash\` configuration from your \`vercel.json\` file or Astro config.
`
);
}
} catch (_err) {
Expand Down Expand Up @@ -435,14 +443,12 @@ export default function vercelAdapter({
}
const fourOhFourRoute = routes.find((route) => route.pathname === '/404');
const destination = new URL('./.vercel/output/config.json', _config.root);
const finalRoutes = [
...getRedirects(routes, _config),
const finalRoutes: Route[] = [
{
src: `^/${_config.build.assets}/(.*)$`,
headers: { 'cache-control': 'public, max-age=31536000, immutable' },
continue: true,
},
{ handle: 'filesystem' },
];
if (_buildOutput === 'server') {
finalRoutes.push(...routeDefinitions);
Expand All @@ -468,6 +474,23 @@ export default function vercelAdapter({
}
}

let trailingSlash: boolean | undefined;

if (_config.trailingSlash && _config.trailingSlash !== 'ignore') {
trailingSlash = _config.trailingSlash === 'always';
ematipico marked this conversation as resolved.
Show resolved Hide resolved
}

const { routes: redirects = [], error } = getTransformedRoutes({
trailingSlash,
rewrites: [],
redirects: getRedirects(routes, _config),
headers: [],
});

if (error) {
logger.error(`Error generating redirects: ${error.message}`);
ascorbic marked this conversation as resolved.
Show resolved Hide resolved
}

let images: VercelImageConfig | undefined;
if (imageService || imagesConfig) {
if (imagesConfig) {
Expand All @@ -487,11 +510,16 @@ export default function vercelAdapter({
}
}

const normalized = normalizeRoutes([...(redirects ?? []), ...finalRoutes]);
if (normalized.error) {
logger.error(`Error creating routes: ${normalized.error.message}`);
ascorbic marked this conversation as resolved.
Show resolved Hide resolved
}

// Output configuration
// https://vercel.com/docs/build-output-api/v3#build-output-configuration
await writeJson(destination, {
version: 3,
routes: finalRoutes,
routes: [...(redirects ?? []), ...finalRoutes],
images,
});

Expand Down
91 changes: 39 additions & 52 deletions packages/vercel/src/lib/redirects.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import nodePath from 'node:path';
import { appendForwardSlash, removeLeadingForwardSlash } from '@astrojs/internal-helpers/path';
import { removeLeadingForwardSlash } from '@astrojs/internal-helpers/path';
import type { AstroConfig, IntegrationResolvedRoute, RoutePart } from 'astro';

import type { Redirect } from '@vercel/routing-utils';

const pathJoin = nodePath.posix.join;

// https://vercel.com/docs/project-configuration#legacy/routes
Expand Down Expand Up @@ -40,10 +42,32 @@ function getParts(part: string, file: string) {

return result;
}
/**
* Convert Astro routes into Vercel path-to-regexp syntax, which are the input for getTransformedRoutes
*/
function getMatchPattern(segments: RoutePart[][]) {
return segments
.map((segment) => {
return segment
.map((part) => {
if (part.spread) {
// Extract parameter name from spread syntax (e.g., "...slug" -> "slug")
const paramName = part.content.startsWith('...') ? part.content.slice(3) : part.content;
return `:${paramName}*`;
}
if (part.dynamic) {
return `:${part.content}`;
}
return part.content;
})
.join('');
})
.join('/');
}

// Copied from /home/juanm04/dev/misc/astro/packages/astro/src/core/routing/manifest/create.ts
// 2022-04-26
function getMatchPattern(segments: RoutePart[][]) {
function getMatchRegex(segments: RoutePart[][]) {
return segments
.map((segment, segmentIndex) => {
return segment.length === 1 && segment[0].spread
Expand Down Expand Up @@ -72,37 +96,17 @@ function getMatchPattern(segments: RoutePart[][]) {
.join('');
}

function getReplacePattern(segments: RoutePart[][]) {
let n = 0;
let result = '';

for (const segment of segments) {
for (const part of segment) {
// biome-ignore lint/style/useTemplate: <explanation>
if (part.dynamic) result += '$' + ++n;
else result += part.content;
}
result += '/';
}

// Remove trailing slash
result = result.slice(0, -1);

return result;
}

function getRedirectLocation(route: IntegrationResolvedRoute, config: AstroConfig): string {
if (route.redirectRoute) {
const pattern = getReplacePattern(route.redirectRoute.segments);
const path = config.trailingSlash === 'always' ? appendForwardSlash(pattern) : pattern;
return pathJoin(config.base, path);
// biome-ignore lint/style/noUselessElse: <explanation>
} else if (typeof route.redirect === 'object') {
const pattern = getMatchPattern(route.redirectRoute.segments);
return pathJoin(config.base, pattern);
}

if (typeof route.redirect === 'object') {
return pathJoin(config.base, route.redirect.destination);
// biome-ignore lint/style/noUselessElse: <explanation>
} else {
return pathJoin(config.base, route.redirect || '');
}
return pathJoin(config.base, route.redirect || '');
}

function getRedirectStatus(route: IntegrationResolvedRoute): number {
Expand All @@ -119,40 +123,23 @@ export function escapeRegex(content: string) {
.map((s: string) => {
return getParts(s, content);
});
return `^/${getMatchPattern(segments)}$`;
return `^/${getMatchRegex(segments)}$`;
}

export function getRedirects(
routes: IntegrationResolvedRoute[],
config: AstroConfig
): VercelRoute[] {
const redirects: VercelRoute[] = [];
): Redirect[] {
const redirects: Redirect[] = [];

for (const route of routes) {
if (route.type === 'redirect') {
redirects.push({
src: config.base + getMatchPattern(route.segments),
headers: { Location: getRedirectLocation(route, config) },
status: getRedirectStatus(route),
source: config.base + getMatchPattern(route.segments),
destination: getRedirectLocation(route, config),
statusCode: getRedirectStatus(route),
});
} else if (route.type === 'page' && route.pattern !== '/') {
if (config.trailingSlash === 'always') {
redirects.push({
src: config.base + getMatchPattern(route.segments),
// biome-ignore lint/style/useTemplate: <explanation>
headers: { Location: config.base + getReplacePattern(route.segments) + '/' },
status: 308,
});
} else if (config.trailingSlash === 'never') {
redirects.push({
// biome-ignore lint/style/useTemplate: <explanation>
src: config.base + getMatchPattern(route.segments) + '/',
headers: { Location: config.base + getReplacePattern(route.segments) },
status: 308,
});
}
}
}

return redirects;
}
};
27 changes: 5 additions & 22 deletions packages/vercel/test/redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,30 +27,28 @@ describe('Redirects', () => {
async function getConfig() {
const json = await fixture.readFile('../.vercel/output/config.json');
const config = JSON.parse(json);

return config;
}

it('define static routes', async () => {
const config = await getConfig();

const oneRoute = config.routes.find((r) => r.src === '/one');
const oneRoute = config.routes.find((r) => r.src === '^/one$');
assert.equal(oneRoute.headers.Location, '/');
assert.equal(oneRoute.status, 301);

const twoRoute = config.routes.find((r) => r.src === '/two');
const twoRoute = config.routes.find((r) => r.src === '^/two$');
assert.equal(twoRoute.headers.Location, '/');
assert.equal(twoRoute.status, 301);

const threeRoute = config.routes.find((r) => r.src === '/three');
const threeRoute = config.routes.find((r) => r.src === '^/three$');
assert.equal(threeRoute.headers.Location, '/');
assert.equal(threeRoute.status, 302);
});

it('define redirects for static files', async () => {
const config = await getConfig();

const staticRoute = config.routes.find((r) => r.src === '/Basic/http-2-0.html');
const staticRoute = config.routes.find((r) => r.src === '^/Basic/http-2-0\\.html$');
assert.notEqual(staticRoute, undefined);
assert.equal(staticRoute.headers.Location, '/posts/http2');
assert.equal(staticRoute.status, 301);
Expand All @@ -59,25 +57,10 @@ describe('Redirects', () => {
it('defines dynamic routes', async () => {
const config = await getConfig();

const blogRoute = config.routes.find((r) => r.src.startsWith('/blog'));
const blogRoute = config.routes.find((r) => r.src.startsWith('^/blog'));
assert.notEqual(blogRoute, undefined);
assert.equal(blogRoute.headers.Location.startsWith('/team/articles'), true);
assert.equal(blogRoute.status, 301);
});

it('define trailingSlash redirect for sub pages', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed because this isn't how they're done now: it's a global thing instead

const config = await getConfig();

const subpathRoute = config.routes.find((r) => r.src === '/subpage');
assert.notEqual(subpathRoute, undefined);
assert.equal(subpathRoute.headers.Location, '/subpage/');
});

it('does not define trailingSlash redirect for root page', async () => {
const config = await getConfig();
assert.equal(
config.routes.find((r) => r.src === '/'),
undefined
);
});
});
17 changes: 17 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading