From 6ef9a6f958b089ac2890f94a015c978e0b254bda Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Thu, 30 Jan 2025 11:06:59 +0000 Subject: [PATCH 1/2] fix: use vercel routing utils (#525) * fix: use vercel routing utils * changeset * log error * Format * Update changeset * Update tests * Changes from review * Format --- .changeset/tidy-walls-check.md | 5 + packages/vercel/package.json | 1 + packages/vercel/src/index.ts | 65 ++++++++++--- packages/vercel/src/lib/redirects.ts | 93 ++++++++----------- packages/vercel/test/isr.test.js | 14 +-- .../test/prerendered-error-pages.test.js | 2 +- packages/vercel/test/redirects.test.js | 39 ++++---- packages/vercel/test/static.test.js | 2 +- pnpm-lock.yaml | 17 ++++ 9 files changed, 141 insertions(+), 97 deletions(-) create mode 100644 .changeset/tidy-walls-check.md diff --git a/.changeset/tidy-walls-check.md b/.changeset/tidy-walls-check.md new file mode 100644 index 000000000..4cd980301 --- /dev/null +++ b/.changeset/tidy-walls-check.md @@ -0,0 +1,5 @@ +--- +'@astrojs/vercel': patch +--- + +Fixes a bug that caused redirect loops when trailingSlash was set diff --git a/packages/vercel/package.json b/packages/vercel/package.json index df33e7690..f6ebda8ca 100644 --- a/packages/vercel/package.json +++ b/packages/vercel/package.json @@ -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" }, diff --git a/packages/vercel/src/index.ts b/packages/vercel/src/index.ts index 6b46512ec..3ee954202 100644 --- a/packages/vercel/src/index.ts +++ b/packages/vercel/src/index.ts @@ -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, @@ -10,6 +11,7 @@ import type { HookParameters, IntegrationResolvedRoute, } from 'astro'; +import { AstroError } from 'astro/errors'; import glob from 'fast-glob'; import { type DevImageService, @@ -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: - `\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) { @@ -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); @@ -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'; + } + + 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) { @@ -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, }); diff --git a/packages/vercel/src/lib/redirects.ts b/packages/vercel/src/lib/redirects.ts index a9a3a81b2..2b51c9007 100644 --- a/packages/vercel/src/lib/redirects.ts +++ b/packages/vercel/src/lib/redirects.ts @@ -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 @@ -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 @@ -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: - 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: - } 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: - } else { - return pathJoin(config.base, route.redirect || ''); } + return pathJoin(config.base, route.redirect || ''); } function getRedirectStatus(route: IntegrationResolvedRoute): number { @@ -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: - headers: { Location: config.base + getReplacePattern(route.segments) + '/' }, - status: 308, - }); - } else if (config.trailingSlash === 'never') { - redirects.push({ - // biome-ignore lint/style/useTemplate: - src: config.base + getMatchPattern(route.segments) + '/', - headers: { Location: config.base + getReplacePattern(route.segments) }, - status: 308, - }); - } } } - return redirects; } diff --git a/packages/vercel/test/isr.test.js b/packages/vercel/test/isr.test.js index 2e7c17597..10da61192 100644 --- a/packages/vercel/test/isr.test.js +++ b/packages/vercel/test/isr.test.js @@ -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', }, ]); diff --git a/packages/vercel/test/prerendered-error-pages.test.js b/packages/vercel/test/prerendered-error-pages.test.js index 8e8559da0..a20233932 100644 --- a/packages/vercel/test/prerendered-error-pages.test.js +++ b/packages/vercel/test/prerendered-error-pages.test.js @@ -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, } diff --git a/packages/vercel/test/redirects.test.js b/packages/vercel/test/redirects.test.js index 57de308e2..cbdecbef1 100644 --- a/packages/vercel/test/redirects.test.js +++ b/packages/vercel/test/redirects.test.js @@ -27,22 +27,20 @@ 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); }); @@ -50,7 +48,7 @@ describe('Redirects', () => { 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); @@ -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 () => { - 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*".', + }); }); }); diff --git a/packages/vercel/test/static.test.js b/packages/vercel/test/static.test.js index e007862d7..20507f864 100644 --- a/packages/vercel/test/static.test.js +++ b/packages/vercel/test/static.test.js @@ -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, }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 58c9e87b8..2cfa87078 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -529,6 +529,9 @@ importers: '@vercel/nft': specifier: ^0.29.0 version: 0.29.0(rollup@4.30.1) + '@vercel/routing-utils': + specifier: ^5.0.0 + version: 5.0.0 esbuild: specifier: ^0.24.0 version: 0.24.2 @@ -2470,6 +2473,9 @@ packages: engines: {node: '>=18'} hasBin: true + '@vercel/routing-utils@5.0.0': + resolution: {integrity: sha512-llvozDbkGDSelbgigAt9IwCQS8boP4rNHfy3rpJf0DqSn6UDlkFX270NwIQruyXN9KHktHC9qOof6Ik2+bT88A==} + '@vitejs/plugin-vue-jsx@4.1.1': resolution: {integrity: sha512-uMJqv/7u1zz/9NbWAD3XdjaY20tKTf17XVfQ9zq4wY1BjsB/PjpJPMe2xiG39QpP4ZdhYNhm4Hvo66uJrykNLA==} engines: {node: ^18.0.0 || >=20.0.0} @@ -4208,6 +4214,9 @@ packages: path-to-regexp@0.1.12: resolution: {integrity: sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==} + path-to-regexp@6.1.0: + resolution: {integrity: sha512-h9DqehX3zZZDCEm+xbfU0ZmwCGFCAAraPJWMXJ4+v32NjZJilVg3k1TcKsRgIb8IQ/izZSaydDc1OhJCZvs2Dw==} + path-to-regexp@6.3.0: resolution: {integrity: sha512-Yhpw4T9C6hPpgPeA28us07OJeqZ5EzQTkbfwuhsUg0c237RomFoETJgmp2sa3F/41gfLE6G5cqcYwznmeEeOlQ==} @@ -7529,6 +7538,12 @@ snapshots: - rollup - supports-color + '@vercel/routing-utils@5.0.0': + dependencies: + path-to-regexp: 6.1.0 + optionalDependencies: + ajv: 6.12.6 + '@vitejs/plugin-vue-jsx@4.1.1(vite@6.0.7(@types/node@22.10.6)(yaml@2.6.1))(vue@3.5.13(typescript@5.7.3))': dependencies: '@babel/core': 7.26.0 @@ -9710,6 +9725,8 @@ snapshots: path-to-regexp@0.1.12: {} + path-to-regexp@6.1.0: {} + path-to-regexp@6.3.0: {} path-type@4.0.0: {} From 991eea7352b1c15f17a95aadedd78f918065a462 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 30 Jan 2025 11:09:22 +0000 Subject: [PATCH 2/2] [ci] release (#520) Co-authored-by: github-actions[bot] --- .changeset/grumpy-phones-explode.md | 5 ----- .changeset/tidy-walls-check.md | 5 ----- packages/vercel/CHANGELOG.md | 8 ++++++++ packages/vercel/package.json | 2 +- 4 files changed, 9 insertions(+), 11 deletions(-) delete mode 100644 .changeset/grumpy-phones-explode.md delete mode 100644 .changeset/tidy-walls-check.md diff --git a/.changeset/grumpy-phones-explode.md b/.changeset/grumpy-phones-explode.md deleted file mode 100644 index db2db6cef..000000000 --- a/.changeset/grumpy-phones-explode.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@astrojs/vercel': patch ---- - -Updates edge middleware to support esnext syntax diff --git a/.changeset/tidy-walls-check.md b/.changeset/tidy-walls-check.md deleted file mode 100644 index 4cd980301..000000000 --- a/.changeset/tidy-walls-check.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@astrojs/vercel': patch ---- - -Fixes a bug that caused redirect loops when trailingSlash was set diff --git a/packages/vercel/CHANGELOG.md b/packages/vercel/CHANGELOG.md index acac6ec4e..a2a237542 100644 --- a/packages/vercel/CHANGELOG.md +++ b/packages/vercel/CHANGELOG.md @@ -1,5 +1,13 @@ # @astrojs/vercel +## 8.0.5 + +### Patch Changes + +- [#519](https://github.com/withastro/adapters/pull/519) [`641d7d5`](https://github.com/withastro/adapters/commit/641d7d588d2d77f519201e583f0275db4260575c) Thanks [@ascorbic](https://github.com/ascorbic)! - Updates edge middleware to support esnext syntax + +- [#525](https://github.com/withastro/adapters/pull/525) [`6ef9a6f`](https://github.com/withastro/adapters/commit/6ef9a6f958b089ac2890f94a015c978e0b254bda) Thanks [@ascorbic](https://github.com/ascorbic)! - Fixes a bug that caused redirect loops when trailingSlash was set + ## 8.0.4 ### Patch Changes diff --git a/packages/vercel/package.json b/packages/vercel/package.json index f6ebda8ca..acd0e19cd 100644 --- a/packages/vercel/package.json +++ b/packages/vercel/package.json @@ -1,7 +1,7 @@ { "name": "@astrojs/vercel", "description": "Deploy your site to Vercel", - "version": "8.0.4", + "version": "8.0.5", "type": "module", "author": "withastro", "license": "MIT",