From 103d2e397edc19dd393930d02aec95581e682398 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Mon, 4 Sep 2023 18:13:30 +0200 Subject: [PATCH 1/6] fix: include route prefix in vercel func names --- .../vercel/src/serverless/adapter.ts | 24 ++++++++++++++----- .../astro.config.mjs | 10 ++++++++ .../included.js | 1 + .../package.json | 9 +++++++ .../src/pages/[id]/index.astro | 12 ++++++++++ .../src/pages/api/[id].js | 7 ++++++ .../src/pages/index.astro | 12 ++++++++++ .../serverless-with-dynamic-routes.test.js | 22 +++++++++++++++++ 8 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/astro.config.mjs create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/included.js create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/package.json create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/[id]/index.astro create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/api/[id].js create mode 100644 packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/index.astro create mode 100644 packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js diff --git a/packages/integrations/vercel/src/serverless/adapter.ts b/packages/integrations/vercel/src/serverless/adapter.ts index a7178b3e3c59..baf66fb15e3a 100644 --- a/packages/integrations/vercel/src/serverless/adapter.ts +++ b/packages/integrations/vercel/src/serverless/adapter.ts @@ -208,13 +208,25 @@ You can set functionPerRoute: false to prevent surpassing the limit.` // Multiple entrypoint support if (_entryPoints.size) { for (const [route, entryFile] of _entryPoints) { - const func = basename(entryFile.toString()).replace(/\.mjs$/, ''); + const [entryNameHead, ...entryNameTail] = basename(entryFile.toString()).split('.'); + const entryPathSegments = []; + + for (let i = 0; i < route.segments.length - 1; i++) { + const segment = route.segments[i][0]; + entryPathSegments.push(segment.content) + } + + // Include route segment in the func name to avoid naming conflicts + // See: https://github.com/withastro/astro/issues/8401 + const func = [entryNameHead, ...entryPathSegments, ...entryNameTail].join('.') + .replace(/\.mjs$/, "") + await createFunctionFolder(func, entryFile, filesToInclude, logger); - routeDefinitions.push({ - src: route.pattern.source, - dest: func, - }); - } + routeDefinitions.push({ + src: route.pattern.source, + dest: func, + }); + } } else { await createFunctionFolder( 'render', diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/astro.config.mjs b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/astro.config.mjs new file mode 100644 index 000000000000..f5a86e609939 --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/astro.config.mjs @@ -0,0 +1,10 @@ +import { defineConfig } from 'astro/config'; +import vercel from '@astrojs/vercel/serverless'; + +export default defineConfig({ + adapter: vercel({ + // Pass some value to make sure it doesn't error out + includeFiles: ['included.js'], + }), + output: 'server' +}); diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/included.js b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/included.js new file mode 100644 index 000000000000..4e64b2d616bf --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/included.js @@ -0,0 +1 @@ +'works' \ No newline at end of file diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/package.json b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/package.json new file mode 100644 index 000000000000..e22a7e932aea --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/package.json @@ -0,0 +1,9 @@ +{ + "name": "@test/astro-vercel-serverless-with-dynamic-routes", + "version": "0.0.0", + "private": true, + "dependencies": { + "@astrojs/vercel": "workspace:*", + "astro": "workspace:*" + } +} diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/[id]/index.astro b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/[id]/index.astro new file mode 100644 index 000000000000..4eab5952d650 --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/[id]/index.astro @@ -0,0 +1,12 @@ +--- +export const prerender = false; +--- + + + + testing {Astro.params.id} + + +

testing {Astro.params.id}

+ + diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/api/[id].js b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/api/[id].js new file mode 100644 index 000000000000..f54e673a8919 --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/api/[id].js @@ -0,0 +1,7 @@ +export const prerender = false; + +export async function GET({ params }) { + return Response.json({ + id: params.id + }); +} diff --git a/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/index.astro b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/index.astro new file mode 100644 index 000000000000..b6b833e5358f --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes/src/pages/index.astro @@ -0,0 +1,12 @@ +--- +export const prerender = import.meta.env.PRERENDER; +--- + + + + testing + + +

testing

+ + diff --git a/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js b/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js new file mode 100644 index 000000000000..f5856ed77a27 --- /dev/null +++ b/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js @@ -0,0 +1,22 @@ +import { expect } from 'chai'; +import { loadFixture } from './test-utils.js'; + +describe('Serverless with dynamic routes', () => { + /** @type {import('./test-utils.js').Fixture} */ + let fixture; + + before(async () => { + process.env.PRERENDER = true; + fixture = await loadFixture({ + root: './fixtures/serverless-with-dynamic-routes/', + output: 'hybrid', + }); + await fixture.build(); + }); + + it('build successful', async () => { + expect(await fixture.readFile('../.vercel/output/static/index.html')).to.be.ok; + expect(await fixture.readFile('../.vercel/output/functions/entry._id_.astro.func/.vc-config.json')).to.be.ok; + expect(await fixture.readFile('../.vercel/output/functions/entry.api._id_.astro.func/.vc-config.json')).to.be.ok; + }); +}); From d690a2b3bdd274d98fd6567d058e941b3df74753 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Mon, 4 Sep 2023 18:20:37 +0200 Subject: [PATCH 2/6] chore: add changeset --- .changeset/modern-guests-float.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/modern-guests-float.md diff --git a/.changeset/modern-guests-float.md b/.changeset/modern-guests-float.md new file mode 100644 index 000000000000..cda74642aac0 --- /dev/null +++ b/.changeset/modern-guests-float.md @@ -0,0 +1,5 @@ +--- +'@astrojs/vercel': patch +--- + +Fix serverless function naming conflicts for routes with identical filenames but different directory structures From 2e47d36cca679f8e6473d8aa6d0f779b06675c53 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Mon, 4 Sep 2023 18:28:44 +0200 Subject: [PATCH 3/6] chore: update pnpm lockfile --- pnpm-lock.yaml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9efde990009f..9888b0abf697 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -4800,6 +4800,15 @@ importers: specifier: workspace:* version: link:../../../../../astro + packages/integrations/vercel/test/fixtures/serverless-with-dynamic-routes: + dependencies: + '@astrojs/vercel': + specifier: workspace:* + version: link:../../.. + astro: + specifier: workspace:* + version: link:../../../../../astro + packages/integrations/vercel/test/fixtures/static-assets: dependencies: '@astrojs/vercel': From ce3d318b38d1dbceee671dd6c1c219364f1af553 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Tue, 5 Sep 2023 14:48:54 +0200 Subject: [PATCH 4/6] refactor: simplify logic that generates vercel func names --- .../vercel/src/serverless/adapter.ts | 31 +++++++++++-------- .../serverless-with-dynamic-routes.test.js | 4 +-- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/integrations/vercel/src/serverless/adapter.ts b/packages/integrations/vercel/src/serverless/adapter.ts index baf66fb15e3a..8497ec68782e 100644 --- a/packages/integrations/vercel/src/serverless/adapter.ts +++ b/packages/integrations/vercel/src/serverless/adapter.ts @@ -207,22 +207,27 @@ You can set functionPerRoute: false to prevent surpassing the limit.` // Multiple entrypoint support if (_entryPoints.size) { - for (const [route, entryFile] of _entryPoints) { - const [entryNameHead, ...entryNameTail] = basename(entryFile.toString()).split('.'); - const entryPathSegments = []; + // TODO: Currently this function directly checks for a single route + // injected in packages/astro/src/assets/internal.ts Ideally this + // should be done dynamically + const isInjectedRoute = (route: RouteData) => + route.pathname === '/_image'; - for (let i = 0; i < route.segments.length - 1; i++) { - const segment = route.segments[i][0]; - entryPathSegments.push(segment.content) - } - - // Include route segment in the func name to avoid naming conflicts - // See: https://github.com/withastro/astro/issues/8401 - const func = [entryNameHead, ...entryPathSegments, ...entryNameTail].join('.') - .replace(/\.mjs$/, "") + const getInjectedFuncName = (entryFile: URL) => + basename(entryFile.toString()) + .replace(/^entry-/, '') + .replace(/\.mjs$/, ''); + + const getFuncName = (route: RouteData) => + route.component.replace(/^src\/pages\//, '') + for (const [route, entryFile] of _entryPoints) { + const func = isInjectedRoute(route) + ? getInjectedFuncName(entryFile) + : getFuncName(route) + await createFunctionFolder(func, entryFile, filesToInclude, logger); - routeDefinitions.push({ + routeDefinitions.push({ src: route.pattern.source, dest: func, }); diff --git a/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js b/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js index f5856ed77a27..325f9c5b0460 100644 --- a/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js +++ b/packages/integrations/vercel/test/serverless-with-dynamic-routes.test.js @@ -16,7 +16,7 @@ describe('Serverless with dynamic routes', () => { it('build successful', async () => { expect(await fixture.readFile('../.vercel/output/static/index.html')).to.be.ok; - expect(await fixture.readFile('../.vercel/output/functions/entry._id_.astro.func/.vc-config.json')).to.be.ok; - expect(await fixture.readFile('../.vercel/output/functions/entry.api._id_.astro.func/.vc-config.json')).to.be.ok; + expect(await fixture.readFile('../.vercel/output/functions/[id]/index.astro.func/.vc-config.json')).to.be.ok; + expect(await fixture.readFile('../.vercel/output/functions/api/[id].js.func/.vc-config.json')).to.be.ok; }); }); From 0d344d4f4242cf1f8992f633b25ee91d13b01a77 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Tue, 5 Sep 2023 14:53:46 +0200 Subject: [PATCH 5/6] fix: properly remove entryFile prefix from func name --- packages/integrations/vercel/src/serverless/adapter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integrations/vercel/src/serverless/adapter.ts b/packages/integrations/vercel/src/serverless/adapter.ts index 8497ec68782e..fd7276b87cf8 100644 --- a/packages/integrations/vercel/src/serverless/adapter.ts +++ b/packages/integrations/vercel/src/serverless/adapter.ts @@ -215,7 +215,7 @@ You can set functionPerRoute: false to prevent surpassing the limit.` const getInjectedFuncName = (entryFile: URL) => basename(entryFile.toString()) - .replace(/^entry-/, '') + .replace(/^entry\./, '') .replace(/\.mjs$/, ''); const getFuncName = (route: RouteData) => From 07ee266b8418d63f05aa7fcf777a46bd9751d036 Mon Sep 17 00:00:00 2001 From: Slawek Kolodziej Date: Tue, 5 Sep 2023 19:54:29 +0200 Subject: [PATCH 6/6] refactor: change how vercel function names are generated --- .../vercel/src/serverless/adapter.ts | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/integrations/vercel/src/serverless/adapter.ts b/packages/integrations/vercel/src/serverless/adapter.ts index fd7276b87cf8..32735da03bd6 100644 --- a/packages/integrations/vercel/src/serverless/adapter.ts +++ b/packages/integrations/vercel/src/serverless/adapter.ts @@ -207,24 +207,18 @@ You can set functionPerRoute: false to prevent surpassing the limit.` // Multiple entrypoint support if (_entryPoints.size) { - // TODO: Currently this function directly checks for a single route - // injected in packages/astro/src/assets/internal.ts Ideally this - // should be done dynamically - const isInjectedRoute = (route: RouteData) => - route.pathname === '/_image'; + const getRouteFuncName = (route: RouteData) => + route.component.replace('src/pages/', '') - const getInjectedFuncName = (entryFile: URL) => + const getFallbackFuncName = (entryFile: URL) => basename(entryFile.toString()) - .replace(/^entry\./, '') + .replace('entry.', '') .replace(/\.mjs$/, ''); - const getFuncName = (route: RouteData) => - route.component.replace(/^src\/pages\//, '') - for (const [route, entryFile] of _entryPoints) { - const func = isInjectedRoute(route) - ? getInjectedFuncName(entryFile) - : getFuncName(route) + const func = route.component.startsWith('src/pages/') + ? getRouteFuncName(route) + : getFallbackFuncName(entryFile) await createFunctionFolder(func, entryFile, filesToInclude, logger); routeDefinitions.push({