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 all 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 @@ -10,6 +11,7 @@ import type {
HookParameters,
IntegrationResolvedRoute,
} from 'astro';
import { AstroError } from 'astro/errors';
import glob from 'fast-glob';
import {
type DevImageService,
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