From 3ab0f5b49b88199e8ae07113d4fad233d7a6b641 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 18 Jul 2024 09:49:11 -0400 Subject: [PATCH 1/5] Fix Server Islands in Vercel --- packages/astro/src/core/build/index.ts | 6 +++- .../astro/src/core/server-islands/endpoint.ts | 35 +++++++++---------- .../fixtures/server-islands/astro.config.mjs | 10 ++++++ .../test/fixtures/server-islands/package.json | 10 ++++++ .../src/components/Island.astro | 1 + .../server-islands/src/pages/index.astro | 12 +++++++ .../vercel/test/server-islands.test.js | 29 +++++++++++++++ pnpm-lock.yaml | 9 +++++ 8 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 packages/integrations/vercel/test/fixtures/server-islands/astro.config.mjs create mode 100644 packages/integrations/vercel/test/fixtures/server-islands/package.json create mode 100644 packages/integrations/vercel/test/fixtures/server-islands/src/components/Island.astro create mode 100644 packages/integrations/vercel/test/fixtures/server-islands/src/pages/index.astro create mode 100644 packages/integrations/vercel/test/server-islands.test.js diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 7933b77f9732..58ef2e75ecee 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -32,6 +32,7 @@ import { collectPagesData } from './page-data.js'; import { staticBuild, viteBuild } from './static-build.js'; import type { StaticBuildOptions } from './types.js'; import { getTimeStat } from './util.js'; +import { SERVER_ISLAND_ROUTE_DATA } from '../server-islands/endpoint.js'; export interface BuildOptions { /** @@ -216,7 +217,10 @@ class AstroBuilder { pages: pageNames, routes: Object.values(allPages) .flat() - .map((pageData) => pageData.route), + .map((pageData) => pageData.route).concat( + this.settings.config.experimental.serverIslands ? + [ SERVER_ISLAND_ROUTE_DATA] : [] + ), logging: this.logger, cacheManifest: internals.cacheManifestUsed, }); diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index 7b6857e1ac5c..27c3f74fdefa 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -14,30 +14,29 @@ import { createSlotValueFromString } from '../../runtime/server/render/slot.js'; export const SERVER_ISLAND_ROUTE = '/_server-islands/[name]'; export const SERVER_ISLAND_COMPONENT = '_server-islands.astro'; +export const SERVER_ISLAND_ROUTE_DATA: RouteData = { + type: 'page', + component: SERVER_ISLAND_COMPONENT, + generate: () => '', + params: ['name'], + segments: [ + [{ content: '_server-islands', dynamic: false, spread: false }], + [{ content: 'name', dynamic: true, spread: false }], + ], + // eslint-disable-next-line + pattern: /^\/_server-islands\/([^/]+?)$/, + prerender: false, + isIndex: false, + fallbackRoutes: [], + route: SERVER_ISLAND_ROUTE, +}; export function ensureServerIslandRoute(manifest: ManifestData) { if (manifest.routes.some((route) => route.route === '/_server-islands/[name]')) { return; } - const route: RouteData = { - type: 'page', - component: SERVER_ISLAND_COMPONENT, - generate: () => '', - params: ['name'], - segments: [ - [{ content: '_server-islands', dynamic: false, spread: false }], - [{ content: 'name', dynamic: true, spread: false }], - ], - // eslint-disable-next-line - pattern: /^\/_server-islands\/([^/]+?)$/, - prerender: false, - isIndex: false, - fallbackRoutes: [], - route: SERVER_ISLAND_ROUTE, - }; - - manifest.routes.push(route); + manifest.routes.push(SERVER_ISLAND_ROUTE_DATA); } type RenderOptions = { diff --git a/packages/integrations/vercel/test/fixtures/server-islands/astro.config.mjs b/packages/integrations/vercel/test/fixtures/server-islands/astro.config.mjs new file mode 100644 index 000000000000..534197429c8a --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/server-islands/astro.config.mjs @@ -0,0 +1,10 @@ +import vercel from '@astrojs/vercel/serverless'; +import { defineConfig } from 'astro/config'; + +export default defineConfig({ + output: "server", + adapter: vercel(), + experimental: { + serverIslands: true, + } +}); diff --git a/packages/integrations/vercel/test/fixtures/server-islands/package.json b/packages/integrations/vercel/test/fixtures/server-islands/package.json new file mode 100644 index 000000000000..a21ff176a8ff --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/server-islands/package.json @@ -0,0 +1,10 @@ +{ + "name": "@test/vercel-server-islands", + "version": "0.0.0", + "private": true, + "dependencies": { + "@astrojs/vercel": "workspace:*", + "astro": "workspace:*" + } +} + diff --git a/packages/integrations/vercel/test/fixtures/server-islands/src/components/Island.astro b/packages/integrations/vercel/test/fixtures/server-islands/src/components/Island.astro new file mode 100644 index 000000000000..9d2832bc1878 --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/server-islands/src/components/Island.astro @@ -0,0 +1 @@ +

I'm an island

diff --git a/packages/integrations/vercel/test/fixtures/server-islands/src/pages/index.astro b/packages/integrations/vercel/test/fixtures/server-islands/src/pages/index.astro new file mode 100644 index 000000000000..835126c2bc10 --- /dev/null +++ b/packages/integrations/vercel/test/fixtures/server-islands/src/pages/index.astro @@ -0,0 +1,12 @@ +--- +import Island from '../components/Island.astro'; +--- + + + One + + +

One

+ + + diff --git a/packages/integrations/vercel/test/server-islands.test.js b/packages/integrations/vercel/test/server-islands.test.js new file mode 100644 index 000000000000..060492584874 --- /dev/null +++ b/packages/integrations/vercel/test/server-islands.test.js @@ -0,0 +1,29 @@ +import assert from 'node:assert/strict'; +import { before, describe, it } from 'node:test'; +import { loadFixture } from './test-utils.js'; + +describe('Server Islands', () => { + /** @type {import('./test-utils.js').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/server-islands/', + }); + await fixture.build(); + }); + + it('server islands route is in the config', async () => { + const config = JSON.parse( + await fixture.readFile('../.vercel/output/config.json') + ); + let found = null; + for(let route of config.routes) { + if(route.src?.includes('_server-islands')) { + found = route; + break; + } + } + assert.notEqual(found, null, 'Default server islands route included'); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b73390f55349..33056a56eb3f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -5542,6 +5542,15 @@ importers: specifier: workspace:* version: link:../../../../../astro + packages/integrations/vercel/test/fixtures/server-islands: + dependencies: + '@astrojs/vercel': + specifier: workspace:* + version: link:../../.. + astro: + specifier: workspace:* + version: link:../../../../../astro + packages/integrations/vercel/test/fixtures/serverless-prerender: dependencies: '@astrojs/vercel': From 18fc5ca1c76a9cc4d8abdb654626943f45d22585 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 18 Jul 2024 09:51:07 -0400 Subject: [PATCH 2/5] Add a changeset --- .changeset/tidy-shrimps-grab.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/tidy-shrimps-grab.md diff --git a/.changeset/tidy-shrimps-grab.md b/.changeset/tidy-shrimps-grab.md new file mode 100644 index 000000000000..55e52375e50a --- /dev/null +++ b/.changeset/tidy-shrimps-grab.md @@ -0,0 +1,7 @@ +--- +'astro': patch +--- + +Fix for Server Islands in Vercel adapter + +Vercel, and probably other adapters only allow pre-defined routes. This makes it so that the `astro:build:done` hook includes the `_server-islands/` route as part of the route data, which is used to configure available routes. From e98b45b35dc95a3bf634c3621e89538323b9c56b Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 18 Jul 2024 10:26:40 -0400 Subject: [PATCH 3/5] Get server islands pattern from the segments --- packages/astro/src/core/app/index.ts | 2 +- packages/astro/src/core/build/index.ts | 4 +- packages/astro/src/core/routing/default.ts | 8 ++-- .../astro/src/core/server-islands/endpoint.ts | 43 +++++++++++-------- .../src/vite-plugin-astro-server/plugin.ts | 4 +- 5 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 24bb7b2b7f93..e14630224c09 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -87,7 +87,7 @@ export class App { constructor(manifest: SSRManifest, streaming = true) { this.#manifest = manifest; - this.#manifestData = injectDefaultRoutes({ + this.#manifestData = injectDefaultRoutes(manifest, { routes: manifest.routes.map((route) => route.routeData), }); this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base); diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index 58ef2e75ecee..23e7b7835470 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -32,7 +32,7 @@ import { collectPagesData } from './page-data.js'; import { staticBuild, viteBuild } from './static-build.js'; import type { StaticBuildOptions } from './types.js'; import { getTimeStat } from './util.js'; -import { SERVER_ISLAND_ROUTE_DATA } from '../server-islands/endpoint.js'; +import { getServerIslandRouteData } from '../server-islands/endpoint.js'; export interface BuildOptions { /** @@ -219,7 +219,7 @@ class AstroBuilder { .flat() .map((pageData) => pageData.route).concat( this.settings.config.experimental.serverIslands ? - [ SERVER_ISLAND_ROUTE_DATA] : [] + [ getServerIslandRouteData(this.settings.config) ] : [] ), logging: this.logger, cacheManifest: internals.cacheManifestUsed, diff --git a/packages/astro/src/core/routing/default.ts b/packages/astro/src/core/routing/default.ts index 5bad66ec4f17..12e6fd494e33 100644 --- a/packages/astro/src/core/routing/default.ts +++ b/packages/astro/src/core/routing/default.ts @@ -12,10 +12,10 @@ import { ensure404Route, } from './astro-designed-error-pages.js'; -export function injectDefaultRoutes(manifest: ManifestData) { - ensure404Route(manifest); - ensureServerIslandRoute(manifest); - return manifest; +export function injectDefaultRoutes(ssrManifest: SSRManifest, routeManifest: ManifestData) { + ensure404Route(routeManifest); + ensureServerIslandRoute(ssrManifest, routeManifest); + return routeManifest; } type DefaultRouteParams = { diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index 27c3f74fdefa..683236f687e4 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -11,32 +11,41 @@ import { renderTemplate, } from '../../runtime/server/index.js'; import { createSlotValueFromString } from '../../runtime/server/render/slot.js'; +import { getPattern } from '../routing/manifest/create.js'; export const SERVER_ISLAND_ROUTE = '/_server-islands/[name]'; export const SERVER_ISLAND_COMPONENT = '_server-islands.astro'; -export const SERVER_ISLAND_ROUTE_DATA: RouteData = { - type: 'page', - component: SERVER_ISLAND_COMPONENT, - generate: () => '', - params: ['name'], - segments: [ + +type ConfigFields = Pick; + +export function getServerIslandRouteData(config: ConfigFields) { + const segments = [ [{ content: '_server-islands', dynamic: false, spread: false }], [{ content: 'name', dynamic: true, spread: false }], - ], - // eslint-disable-next-line - pattern: /^\/_server-islands\/([^/]+?)$/, - prerender: false, - isIndex: false, - fallbackRoutes: [], - route: SERVER_ISLAND_ROUTE, -}; + ]; + const route: RouteData = { + type: 'page', + component: SERVER_ISLAND_COMPONENT, + generate: () => '', + params: ['name'], + segments, + pattern: getPattern(segments, config.base, config.trailingSlash), + prerender: false, + isIndex: false, + fallbackRoutes: [], + route: SERVER_ISLAND_ROUTE, + }; + return route; +} + + -export function ensureServerIslandRoute(manifest: ManifestData) { - if (manifest.routes.some((route) => route.route === '/_server-islands/[name]')) { +export function ensureServerIslandRoute(config: ConfigFields, routeManifest: ManifestData) { + if (routeManifest.routes.some((route) => route.route === '/_server-islands/[name]')) { return; } - manifest.routes.push(SERVER_ISLAND_ROUTE_DATA); + routeManifest.routes.push(getServerIslandRouteData(config)); } type RenderOptions = { diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 9904d2844d9a..ccac3fac1485 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -35,7 +35,7 @@ export default function createVitePluginAstroServer({ configureServer(viteServer) { const loader = createViteLoader(viteServer); const manifest = createDevelopmentManifest(settings); - let manifestData: ManifestData = injectDefaultRoutes( + let manifestData: ManifestData = injectDefaultRoutes(manifest, createRouteManifest({ settings, fsMod }, logger) ); const pipeline = DevPipeline.create(manifestData, { loader, logger, manifest, settings }); @@ -46,7 +46,7 @@ export default function createVitePluginAstroServer({ function rebuildManifest(needsManifestRebuild: boolean) { pipeline.clearRouteCache(); if (needsManifestRebuild) { - manifestData = injectDefaultRoutes(createRouteManifest({ settings }, logger)); + manifestData = injectDefaultRoutes(manifest, createRouteManifest({ settings }, logger)); pipeline.setManifestData(manifestData); } } From 689307f3feeb9156bdad75cd2e59aaec70e42ba7 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 18 Jul 2024 10:40:39 -0400 Subject: [PATCH 4/5] Move getPattern so it can be used at runtime --- .../astro/src/core/routing/manifest/create.ts | 56 +----------------- .../src/core/routing/manifest/pattern.ts | 57 +++++++++++++++++++ .../astro/src/core/server-islands/endpoint.ts | 2 +- 3 files changed, 60 insertions(+), 55 deletions(-) create mode 100644 packages/astro/src/core/routing/manifest/pattern.ts diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 4a36c8536d46..b022c383b94b 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -22,6 +22,7 @@ import { removeLeadingForwardSlash, slash } from '../../path.js'; import { resolvePages } from '../../util.js'; import { routeComparator } from '../priority.js'; import { getRouteGenerator } from './generator.js'; +import { getPattern } from './pattern.js'; const require = createRequire(import.meta.url); interface Item { @@ -70,59 +71,6 @@ export function getParts(part: string, file: string) { return result; } -export function getPattern( - segments: RoutePart[][], - base: AstroConfig['base'], - addTrailingSlash: AstroConfig['trailingSlash'] -) { - const pathname = segments - .map((segment) => { - if (segment.length === 1 && segment[0].spread) { - return '(?:\\/(.*?))?'; - } else { - return ( - '\\/' + - segment - .map((part) => { - if (part.spread) { - return '(.*?)'; - } else if (part.dynamic) { - return '([^/]+?)'; - } else { - return part.content - .normalize() - .replace(/\?/g, '%3F') - .replace(/#/g, '%23') - .replace(/%5B/g, '[') - .replace(/%5D/g, ']') - .replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - } - }) - .join('') - ); - } - }) - .join(''); - - const trailing = - addTrailingSlash && segments.length ? getTrailingSlashPattern(addTrailingSlash) : '$'; - let initial = '\\/'; - if (addTrailingSlash === 'never' && base !== '/') { - initial = ''; - } - return new RegExp(`^${pathname || initial}${trailing}`); -} - -function getTrailingSlashPattern(addTrailingSlash: AstroConfig['trailingSlash']): string { - if (addTrailingSlash === 'always') { - return '\\/$'; - } - if (addTrailingSlash === 'never') { - return '$'; - } - return '\\/?$'; -} - export function validateSegment(segment: string, file = '') { if (!file) file = segment; @@ -486,7 +434,7 @@ function isStaticSegment(segment: RoutePart[]) { * For example, `/foo/[bar]` and `/foo/[baz]` or `/foo/[...bar]` and `/foo/[...baz]` * but not `/foo/[bar]` and `/foo/[...baz]`. */ -function detectRouteCollision(a: RouteData, b: RouteData, config: AstroConfig, logger: Logger) { +function detectRouteCollision(a: RouteData, b: RouteData, _config: AstroConfig, logger: Logger) { if (a.type === 'fallback' || b.type === 'fallback') { // If either route is a fallback route, they don't collide. // Fallbacks are always added below other routes exactly to avoid collisions. diff --git a/packages/astro/src/core/routing/manifest/pattern.ts b/packages/astro/src/core/routing/manifest/pattern.ts new file mode 100644 index 000000000000..320d02e20431 --- /dev/null +++ b/packages/astro/src/core/routing/manifest/pattern.ts @@ -0,0 +1,57 @@ +import type { + AstroConfig, + RoutePart, +} from '../../../@types/astro.js'; + +export function getPattern( + segments: RoutePart[][], + base: AstroConfig['base'], + addTrailingSlash: AstroConfig['trailingSlash'] +) { + const pathname = segments + .map((segment) => { + if (segment.length === 1 && segment[0].spread) { + return '(?:\\/(.*?))?'; + } else { + return ( + '\\/' + + segment + .map((part) => { + if (part.spread) { + return '(.*?)'; + } else if (part.dynamic) { + return '([^/]+?)'; + } else { + return part.content + .normalize() + .replace(/\?/g, '%3F') + .replace(/#/g, '%23') + .replace(/%5B/g, '[') + .replace(/%5D/g, ']') + .replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + } + }) + .join('') + ); + } + }) + .join(''); + + const trailing = + addTrailingSlash && segments.length ? getTrailingSlashPattern(addTrailingSlash) : '$'; + let initial = '\\/'; + if (addTrailingSlash === 'never' && base !== '/') { + initial = ''; + } + return new RegExp(`^${pathname || initial}${trailing}`); +} + +function getTrailingSlashPattern(addTrailingSlash: AstroConfig['trailingSlash']): string { + if (addTrailingSlash === 'always') { + return '\\/$'; + } + if (addTrailingSlash === 'never') { + return '$'; + } + return '\\/?$'; +} diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index 683236f687e4..1e01d0828415 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -11,7 +11,7 @@ import { renderTemplate, } from '../../runtime/server/index.js'; import { createSlotValueFromString } from '../../runtime/server/render/slot.js'; -import { getPattern } from '../routing/manifest/create.js'; +import { getPattern } from '../routing/manifest/pattern.js'; export const SERVER_ISLAND_ROUTE = '/_server-islands/[name]'; export const SERVER_ISLAND_COMPONENT = '_server-islands.astro'; From 4c43ab7764212a9123141e52768c55b9f0267519 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 18 Jul 2024 10:44:57 -0400 Subject: [PATCH 5/5] Fix build --- packages/astro/src/container/index.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index 53bc33e4b946..91cdb1e78e93 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -20,7 +20,8 @@ import { Logger } from '../core/logger/core.js'; import { nodeLogDestination } from '../core/logger/node.js'; import { removeLeadingForwardSlash } from '../core/path.js'; import { RenderContext } from '../core/render-context.js'; -import { getParts, getPattern, validateSegment } from '../core/routing/manifest/create.js'; +import { getParts, validateSegment } from '../core/routing/manifest/create.js'; +import { getPattern } from '../core/routing/manifest/pattern.js'; import type { AstroComponentFactory } from '../runtime/server/index.js'; import { ContainerPipeline } from './pipeline.js';