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 7 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
---

Fixes a bug that caused redirect loops when trailingSlash was set
1 change: 1 addition & 0 deletions packages/vercel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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
65 changes: 53 additions & 12 deletions packages/vercel/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { cpSync, existsSync, mkdirSync, readFileSync } from 'node:fs';
import { basename } from 'node:path';
import { pathToFileURL } from 'node:url';
import { emptyDir, removeDir, writeJson } from '@astrojs/internal-helpers/fs';
import { type Route, getTransformedRoutes, normalizeRoutes } from '@vercel/routing-utils';
import type {
AstroAdapter,
AstroConfig,
Expand All @@ -25,6 +26,7 @@ import {
getInjectableWebAnalyticsContent,
} from './lib/web-analytics.js';
import { generateEdgeMiddleware } from './serverless/middleware.js';
import { AstroError } from 'astro/errors';

const PACKAGE_NAME = '@astrojs/vercel';

Expand Down Expand Up @@ -261,16 +263,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 +444,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 @@ -467,6 +474,30 @@ export default function vercelAdapter({
});
}
}
// The Vercel `trailingSlash` option
let trailingSlash: boolean | undefined;
// Vercel's `trailingSlash` option maps to Astro's like so:
// - `true` -> `"always"`
// - `false` -> `"never"`
// - `undefined` -> `"ignore"`
// If config is set to "ignore", we leave it as undefined.
if (_config.trailingSlash && _config.trailingSlash !== 'ignore') {
// Otherwise, map it accordingly.
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) {
throw new AstroError(
`Error generating redirects: ${error.message}`,
error.link ? `${error.action ?? 'More info'}: ${error.link}` : undefined
);
}

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

const normalized = normalizeRoutes([...(redirects ?? []), ...finalRoutes]);
if (normalized.error) {
throw new AstroError(
`Error generating routes: ${normalized.error.message}`,
normalized.error.link
? `${normalized.error.action ?? 'More info'}: ${normalized.error.link}`
: undefined
);
}

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

Expand Down
93 changes: 38 additions & 55 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,16 @@ 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 +122,20 @@ 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[] = [];
export function getRedirects(routes: IntegrationResolvedRoute[], config: AstroConfig): 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;
}
14 changes: 7 additions & 7 deletions packages/vercel/test/isr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,31 @@ describe('ISR', () => {
dest: '_render',
},
{
src: '^/excluded(?:\\/(.*?))?$',
src: '^/excluded(?:/(.*?))?$',
dest: '_render',
},
{
src: '^\\/_server-islands\\/([^/]+?)\\/?$',
src: '^/_server-islands/([^/]+?)/?$',
dest: '_render',
},
{
src: '^\\/_image\\/?$',
src: '^/_image/?$',
dest: '_render',
},
{
src: '^\\/excluded\\/([^/]+?)\\/?$',
src: '^/excluded/([^/]+?)/?$',
dest: '/_isr?x_astro_path=$0',
},
{
src: '^\\/excluded(?:\\/(.*?))?\\/?$',
src: '^/excluded(?:/(.*?))?/?$',
dest: '/_isr?x_astro_path=$0',
},
{
src: '^\\/one\\/?$',
src: '^/one/?$',
dest: '/_isr?x_astro_path=$0',
},
{
src: '^\\/two\\/?$',
src: '^/two/?$',
dest: '/_isr?x_astro_path=$0',
},
]);
Expand Down
2 changes: 1 addition & 1 deletion packages/vercel/test/prerendered-error-pages.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('prerendered error pages routing', () => {
assert.deepEqual(
deploymentConfig.routes.find((r) => r.status === 404),
{
src: '/.*',
src: '^/.*$',
dest: '/404.html',
status: 404,
}
Expand Down
39 changes: 18 additions & 21 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,24 @@ 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
);
it('throws an error for invalid redirects', async () => {
const fails = await loadFixture({
root: './fixtures/redirects/',
redirects: {
// Invalid source syntax
'/blog/(![...slug]': '/team/articles/[...slug]',
},
});
await assert.rejects(() => fails.build(), {
name: 'AstroUserError',
message:
'Error generating redirects: Redirect at index 0 has invalid `source` regular expression "/blog/(!:slug*".',
});
});
});
2 changes: 1 addition & 1 deletion packages/vercel/test/static.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('static routing', () => {
const deploymentConfig = JSON.parse(await fixture.readFile('../.vercel/output/config.json'));
// change the index if necesseary
assert.deepEqual(deploymentConfig.routes[2], {
src: '/.*',
src: '^/.*$',
dest: '/404.html',
status: 404,
});
Expand Down
Loading
Loading