From 7b9d042dde0c6ae74225de208222e0addf5f4989 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Thu, 17 Mar 2022 08:31:01 -0400 Subject: [PATCH] Allow SSR dynamic routes to not implement getStaticPaths (#2815) * Allow SSR dynamic routes to not implement getStaticPaths * Adds a changeset * Update based on code-review comments --- .changeset/spotty-houses-stare.md | 5 ++++ examples/ssr/astro.config.mjs | 2 +- examples/ssr/src/pages/products/[id].astro | 11 +------- packages/astro/src/core/build/index.ts | 1 + packages/astro/src/core/build/page-data.ts | 5 ++-- packages/astro/src/core/render/core.ts | 10 ++++--- packages/astro/src/core/render/route-cache.ts | 22 +++++++++++---- packages/astro/src/core/routing/validation.ts | 8 ++++-- .../src/vite-plugin-astro-server/index.ts | 1 + .../fixtures/ssr-dynamic/src/pages/[id].astro | 11 ++++++++ packages/astro/test/ssr-dynamic.test.js | 28 +++++++++++++++++++ packages/astro/test/test-utils.js | 2 ++ 12 files changed, 81 insertions(+), 25 deletions(-) create mode 100644 .changeset/spotty-houses-stare.md create mode 100644 packages/astro/test/fixtures/ssr-dynamic/src/pages/[id].astro create mode 100644 packages/astro/test/ssr-dynamic.test.js diff --git a/.changeset/spotty-houses-stare.md b/.changeset/spotty-houses-stare.md new file mode 100644 index 000000000000..a3672caf89cb --- /dev/null +++ b/.changeset/spotty-houses-stare.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Allows dynamic routes in SSR to avoid implementing getStaticPaths diff --git a/examples/ssr/astro.config.mjs b/examples/ssr/astro.config.mjs index fd25be136c21..c8e5061a6aa4 100644 --- a/examples/ssr/astro.config.mjs +++ b/examples/ssr/astro.config.mjs @@ -1,4 +1,4 @@ -// @ts-check +import { defineConfig } from 'astro/config'; export default defineConfig({ renderers: ['@astrojs/renderer-svelte'], diff --git a/examples/ssr/src/pages/products/[id].astro b/examples/ssr/src/pages/products/[id].astro index 9c400c2f103d..37fe7e0b4678 100644 --- a/examples/ssr/src/pages/products/[id].astro +++ b/examples/ssr/src/pages/products/[id].astro @@ -2,18 +2,9 @@ import Header from '../../components/Header.astro'; import Container from '../../components/Container.astro'; import AddToCart from '../../components/AddToCart.svelte'; -import { getProducts, getProduct } from '../../api'; +import { getProduct } from '../../api'; import '../../styles/common.css'; -export async function getStaticPaths() { - const products = await getProducts(); - return products.map(product => { - return { - params: { id: product.id.toString() } - } - }); -} - const id = Number(Astro.request.params.id); const product = await getProduct(id); --- diff --git a/packages/astro/src/core/build/index.ts b/packages/astro/src/core/build/index.ts index ece445aa035c..db06851d22f5 100644 --- a/packages/astro/src/core/build/index.ts +++ b/packages/astro/src/core/build/index.ts @@ -83,6 +83,7 @@ class AstroBuilder { origin, routeCache: this.routeCache, viteServer: this.viteServer, + ssr: this.config.buildOptions.experimentalSsr, }); // Filter pages by using conditions based on their frontmatter. diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index 53c1b15d0452..4cf96d388f62 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -17,6 +17,7 @@ export interface CollectPagesDataOptions { origin: string; routeCache: RouteCache; viteServer: ViteDevServer; + ssr: boolean; } export interface CollectPagesDataResult { @@ -109,11 +110,11 @@ export async function collectPagesData(opts: CollectPagesDataOptions): Promise { - const { astroConfig, logging, routeCache, viteServer } = opts; + const { astroConfig, logging, routeCache, ssr, viteServer } = opts; if (!viteServer) throw new Error(`vite.createServer() not called!`); const filePath = new URL(`./${route.component}`, astroConfig.projectRoot); const mod = (await viteServer.ssrLoadModule(fileURLToPath(filePath))) as ComponentInstance; - const result = await callGetStaticPaths(mod, route, false, logging); + const result = await callGetStaticPaths({ mod, route, isValidate: false, logging, ssr }); routeCache.set(route, result); return result; } diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts index 7d969240cd35..f5c05d0a5696 100644 --- a/packages/astro/src/core/render/core.ts +++ b/packages/astro/src/core/render/core.ts @@ -13,6 +13,7 @@ interface GetParamsAndPropsOptions { routeCache: RouteCache; pathname: string; logging: LogOptions; + ssr: boolean; } export const enum GetParamsAndPropsError { @@ -20,7 +21,7 @@ export const enum GetParamsAndPropsError { } export async function getParamsAndProps(opts: GetParamsAndPropsOptions): Promise<[Params, Props] | GetParamsAndPropsError> { - const { logging, mod, route, routeCache, pathname } = opts; + const { logging, mod, route, routeCache, pathname, ssr} = opts; // Handle dynamic routes let params: Params = {}; let pageProps: Props; @@ -37,18 +38,18 @@ export async function getParamsAndProps(opts: GetParamsAndPropsOptions): Promise // TODO(fks): Can we refactor getParamsAndProps() to receive routeCacheEntry // as a prop, and not do a live lookup/populate inside this lower function call. if (!routeCacheEntry) { - routeCacheEntry = await callGetStaticPaths(mod, route, true, logging); + routeCacheEntry = await callGetStaticPaths({ mod, route, isValidate: true, logging, ssr }); routeCache.set(route, routeCacheEntry); } const matchedStaticPath = findPathItemByKey(routeCacheEntry.staticPaths, params); - if (!matchedStaticPath) { + if (!matchedStaticPath && !ssr) { return GetParamsAndPropsError.NoMatchingStaticPath; } // Note: considered using Object.create(...) for performance // Since this doesn't inherit an object's properties, this caused some odd user-facing behavior. // Ex. console.log(Astro.props) -> {}, but console.log(Astro.props.property) -> 'expected value' // Replaced with a simple spread as a compromise - pageProps = matchedStaticPath.props ? { ...matchedStaticPath.props } : {}; + pageProps = matchedStaticPath?.props ? { ...matchedStaticPath.props } : {}; } else { pageProps = {}; } @@ -83,6 +84,7 @@ export async function render(opts: RenderOptions): Promise<{ type: 'html'; html: route, routeCache, pathname, + ssr, }); if (paramsAndPropsRes === GetParamsAndPropsError.NoMatchingStaticPath) { diff --git a/packages/astro/src/core/render/route-cache.ts b/packages/astro/src/core/render/route-cache.ts index 43f41717e133..5ae36fcb0f31 100644 --- a/packages/astro/src/core/render/route-cache.ts +++ b/packages/astro/src/core/render/route-cache.ts @@ -11,19 +11,29 @@ function stringifyParams(params: Params) { return JSON.stringify(params, Object.keys(params).sort()); } -export async function callGetStaticPaths(mod: ComponentInstance, route: RouteData, isValidate: boolean, logging: LogOptions): Promise { - validateGetStaticPathsModule(mod); +interface CallGetStaticPathsOptions { + mod: ComponentInstance; + route: RouteData; + isValidate: boolean; + logging: LogOptions; + ssr: boolean; +} + +export async function callGetStaticPaths({ isValidate, logging, mod, route, ssr}: CallGetStaticPathsOptions): Promise { + validateGetStaticPathsModule(mod, { ssr }); const resultInProgress = { rss: [] as RSS[], }; - const staticPaths: GetStaticPathsResult = await ( - await mod.getStaticPaths!({ + + let staticPaths: GetStaticPathsResult = []; + if(mod.getStaticPaths) { + staticPaths = (await mod.getStaticPaths({ paginate: generatePaginateFunction(route), rss: (data) => { resultInProgress.rss.push(data); }, - }) - ).flat(); + })).flat(); + } const keyedStaticPaths = staticPaths as GetStaticPathsResultKeyed; keyedStaticPaths.keyed = new Map(); diff --git a/packages/astro/src/core/routing/validation.ts b/packages/astro/src/core/routing/validation.ts index d1d19e98fdaf..d8fda32c4398 100644 --- a/packages/astro/src/core/routing/validation.ts +++ b/packages/astro/src/core/routing/validation.ts @@ -2,12 +2,16 @@ import type { ComponentInstance, GetStaticPathsResult } from '../../@types/astro import type { LogOptions } from '../logger'; import { warn } from '../logger.js'; +interface ValidationOptions { + ssr: boolean; +} + /** Throw error for deprecated/malformed APIs */ -export function validateGetStaticPathsModule(mod: ComponentInstance) { +export function validateGetStaticPathsModule(mod: ComponentInstance, { ssr }: ValidationOptions) { if ((mod as any).createCollection) { throw new Error(`[createCollection] deprecated. Please use getStaticPaths() instead.`); } - if (!mod.getStaticPaths) { + if (!mod.getStaticPaths && !ssr) { throw new Error(`[getStaticPaths] getStaticPaths() function is required. Make sure that you \`export\` the function from your component.`); } } diff --git a/packages/astro/src/vite-plugin-astro-server/index.ts b/packages/astro/src/vite-plugin-astro-server/index.ts index 9002163772d8..bc45596dae5b 100644 --- a/packages/astro/src/vite-plugin-astro-server/index.ts +++ b/packages/astro/src/vite-plugin-astro-server/index.ts @@ -152,6 +152,7 @@ async function handleRequest( routeCache, pathname: rootRelativeUrl, logging, + ssr: config.buildOptions.experimentalSsr, }); if (paramsAndPropsRes === GetParamsAndPropsError.NoMatchingStaticPath) { warn(logging, 'getStaticPaths', `Route pattern matched, but no matching static path found. (${pathname})`); diff --git a/packages/astro/test/fixtures/ssr-dynamic/src/pages/[id].astro b/packages/astro/test/fixtures/ssr-dynamic/src/pages/[id].astro new file mode 100644 index 000000000000..b976757e2166 --- /dev/null +++ b/packages/astro/test/fixtures/ssr-dynamic/src/pages/[id].astro @@ -0,0 +1,11 @@ +--- +const val = Number(Astro.request.params.id); +--- + + + Test app + + +

Item { val }

+ + diff --git a/packages/astro/test/ssr-dynamic.test.js b/packages/astro/test/ssr-dynamic.test.js new file mode 100644 index 000000000000..70e3b0aee8d4 --- /dev/null +++ b/packages/astro/test/ssr-dynamic.test.js @@ -0,0 +1,28 @@ +import { expect } from 'chai'; +import { load as cheerioLoad } from 'cheerio'; +import { loadFixture } from './test-utils.js'; + +// Asset bundling +describe('Dynamic pages in SSR', () => { + let fixture; + + before(async () => { + fixture = await loadFixture({ + projectRoot: './fixtures/ssr-dynamic/', + buildOptions: { + experimentalSsr: true, + } + }); + await fixture.build(); + }); + + it('Do not have to implement getStaticPaths', async () => { + const app = await fixture.loadSSRApp(); + const request = new Request("http://example.com/123"); + const route = app.match(request); + const response = await app.render(request, route); + const html = await response.text(); + const $ = cheerioLoad(html); + expect($('h1').text()).to.equal('Item 123'); + }); +}); diff --git a/packages/astro/test/test-utils.js b/packages/astro/test/test-utils.js index c7060dcc7120..494047ae3134 100644 --- a/packages/astro/test/test-utils.js +++ b/packages/astro/test/test-utils.js @@ -6,6 +6,7 @@ import { loadConfig } from '../dist/core/config.js'; import dev from '../dist/core/dev/index.js'; import build from '../dist/core/build/index.js'; import preview from '../dist/core/preview/index.js'; +import { loadApp } from '../dist/core/app/node.js'; import os from 'os'; import stripAnsi from 'strip-ansi'; @@ -86,6 +87,7 @@ export async function loadFixture(inlineConfig) { inlineConfig.devOptions.port = previewServer.port; // update port for fetch return previewServer; }, + loadSSRApp: () => loadApp(new URL('./server/', config.dist)), readFile: (filePath) => fs.promises.readFile(new URL(filePath.replace(/^\//, ''), config.dist), 'utf8'), readdir: (fp) => fs.promises.readdir(new URL(fp.replace(/^\//, ''), config.dist)), clean: () => fs.promises.rm(config.dist, { maxRetries: 10, recursive: true, force: true }),