From 166c9ed6bdabb82db611af39b4436a90bd711e5d Mon Sep 17 00:00:00 2001 From: "Fred K. Schott" Date: Mon, 23 Aug 2021 12:44:49 -0700 Subject: [PATCH] fix issue with multiple getStaticPaths calls during build (#1194) --- .changeset/mighty-clouds-deliver.md | 5 +++ packages/astro/src/@types/astro.ts | 27 ++++++----- packages/astro/src/build.ts | 36 +++++++-------- packages/astro/src/build/bundle/js.ts | 6 +-- packages/astro/src/build/page.ts | 14 +++--- packages/astro/src/build/paginate.ts | 6 +-- packages/astro/src/build/rss.ts | 4 +- packages/astro/src/runtime.ts | 45 ++++++++++--------- .../astro/test/astro-get-static-paths.test.js | 13 ++++++ .../astro-get-static-paths/astro.config.mjs | 6 +++ .../snowpack.config.json | 3 ++ .../src/pages/[...test].astro | 21 +++++++++ 12 files changed, 116 insertions(+), 70 deletions(-) create mode 100644 .changeset/mighty-clouds-deliver.md create mode 100644 packages/astro/test/astro-get-static-paths.test.js create mode 100644 packages/astro/test/fixtures/astro-get-static-paths/astro.config.mjs create mode 100644 packages/astro/test/fixtures/astro-get-static-paths/snowpack.config.json create mode 100644 packages/astro/test/fixtures/astro-get-static-paths/src/pages/[...test].astro diff --git a/.changeset/mighty-clouds-deliver.md b/.changeset/mighty-clouds-deliver.md new file mode 100644 index 000000000..04f74a97c --- /dev/null +++ b/.changeset/mighty-clouds-deliver.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix an issue where getStaticPaths is called multiple times per build diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts index d9b3500de..bd9986a1f 100644 --- a/packages/astro/src/@types/astro.ts +++ b/packages/astro/src/@types/astro.ts @@ -94,10 +94,7 @@ export interface PageDependencies { images: Set; } -export type PaginateFunction = (data: T[], args?: { pageSize?: number }) => PaginatedCollectionResult; - -export type GetStaticPathsResult = { params: Params; props?: Props }[] | { params: Params; props?: Props }[]; -export interface CollectionRSS { +export interface RSSFunctionArgs { /** (required) Title of the RSS Feed */ title: string; /** (required) Description of the RSS Feed */ @@ -127,10 +124,9 @@ export interface CollectionRSS { }[]; } -export interface PaginatedCollectionResult { +export interface PaginatedCollectionProp { /** result */ data: T[]; - /** metadata */ /** the count of the first item on the page, starting from 0 */ start: number; @@ -138,14 +134,12 @@ export interface PaginatedCollectionResult { end: number; /** total number of results */ total: number; - page: { - /** the current page number, starting from 1 */ - current: number; - /** number of items per page (default: 25) */ - size: number; - /** number of last page */ - last: number; - }; + /** the current page number, starting from 1 */ + currentPage: number; + /** number of items per page (default: 25) */ + size: number; + /** number of last page */ + lastPage: number; url: { /** url of the current page */ current: string; @@ -156,6 +150,11 @@ export interface PaginatedCollectionResult { }; } +export type RSSFunction = (args: RSSFunctionArgs) => void; +export type PaginateFunction = (data: [], args?: { pageSize?: number; params?: Params; props?: Props }) => GetStaticPathsResult; +export type GetStaticPathsArgs = { paginate: PaginateFunction; rss: RSSFunction }; +export type GetStaticPathsResult = { params: Params; props?: Props }[] | { params: Params; props?: Props }[]; + export interface ComponentInfo { url: string; importSpecifier: ImportSpecifier | ImportDefaultSpecifier | ImportNamespaceSpecifier; diff --git a/packages/astro/src/build.ts b/packages/astro/src/build.ts index 7b7dec706..5fc32ec2e 100644 --- a/packages/astro/src/build.ts +++ b/packages/astro/src/build.ts @@ -48,8 +48,8 @@ export async function build(astroConfig: AstroConfig, logging: LogOptions = defa } const mode: RuntimeMode = 'production'; - const runtime = await createRuntime(astroConfig, { mode, logging: runtimeLogging }); - const { runtimeConfig } = runtime; + const astroRuntime = await createRuntime(astroConfig, { mode, logging: runtimeLogging }); + const { runtimeConfig } = astroRuntime; const { snowpackRuntime } = runtimeConfig; try { @@ -69,6 +69,7 @@ export async function build(astroConfig: AstroConfig, logging: LogOptions = defa } else { const result = await getStaticPathsForPage({ astroConfig, + astroRuntime, route, snowpackRuntime, logging, @@ -89,20 +90,17 @@ export async function build(astroConfig: AstroConfig, logging: LogOptions = defa }) ); try { - // TODO: 2x Promise.all? Might be hard to debug + overwhelm resources. await Promise.all( allRoutesAndPaths.map(async ([route, paths]: [RouteData, string[]]) => { - await Promise.all( - paths.map((p) => - buildStaticPage({ - astroConfig, - buildState, - route, - path: p, - astroRuntime: runtime, - }) - ) - ); + for (const p of paths) { + await buildStaticPage({ + astroConfig, + buildState, + route, + path: p, + astroRuntime, + }); + } }) ); } catch (e) { @@ -126,7 +124,7 @@ ${stack} } error(logging, 'build', red('✕ building pages failed!')); - await runtime.shutdown(); + await astroRuntime.shutdown(); return 1; } info(logging, 'build', green('✔'), 'pages built.'); @@ -149,7 +147,7 @@ ${stack} for (const url of [...pageDeps.js, ...pageDeps.css, ...pageDeps.images]) { if (!buildState[url]) scanPromises.push( - runtime.load(url).then((result) => { + astroRuntime.load(url).then((result) => { if (result.statusCode !== 200) { if (result.statusCode === 404) { throw new Error(`${buildState[id].srcPath.href}: could not find "${path.basename(url)}"`); @@ -251,7 +249,7 @@ ${stack} info(logging, 'build', yellow(`! bundling...`)); if (jsImports.size > 0) { timer.bundleJS = performance.now(); - const jsStats = await bundleJS(jsImports, { dist: new URL(dist + '/', projectRoot), runtime }); + const jsStats = await bundleJS(jsImports, { dist: new URL(dist + '/', projectRoot), astroRuntime }); mapBundleStatsToURLStats({ urlStats, depTree, bundleStats: jsStats }); debug(logging, 'build', `bundled JS [${stopTimer(timer.bundleJS)}]`); info(logging, 'build', green(`✔`), 'bundling complete.'); @@ -261,12 +259,12 @@ ${stack} * 6. Print stats */ logURLStats(logging, urlStats); - await runtime.shutdown(); + await astroRuntime.shutdown(); info(logging, 'build', bold(green('▶ Build Complete!'))); return 0; } catch (err) { error(logging, 'build', err.message); - await runtime.shutdown(); + await astroRuntime.shutdown(); return 1; } } diff --git a/packages/astro/src/build/bundle/js.ts b/packages/astro/src/build/bundle/js.ts index d1d651e24..7340d396b 100644 --- a/packages/astro/src/build/bundle/js.ts +++ b/packages/astro/src/build/bundle/js.ts @@ -9,7 +9,7 @@ import { createBundleStats, addBundleStats, BundleStatsMap } from '../stats.js'; interface BundleOptions { dist: URL; - runtime: AstroRuntime; + astroRuntime: AstroRuntime; } /** Collect JS imports from build output */ @@ -22,7 +22,7 @@ export function collectJSImports(buildState: BuildOutput): Set { } /** Bundle JS action */ -export async function bundleJS(imports: Set, { runtime, dist }: BundleOptions): Promise { +export async function bundleJS(imports: Set, { astroRuntime, dist }: BundleOptions): Promise { const ROOT = 'astro:root'; const root = ` ${[...imports].map((url) => `import '${url}';`).join('\n')} @@ -53,7 +53,7 @@ export async function bundleJS(imports: Set, { runtime, dist }: BundleOp return root; } - const result = await runtime.load(id); + const result = await astroRuntime.load(id); if (result.statusCode !== 200) { return null; diff --git a/packages/astro/src/build/page.ts b/packages/astro/src/build/page.ts index 73bb9a189..fed683949 100644 --- a/packages/astro/src/build/page.ts +++ b/packages/astro/src/build/page.ts @@ -1,7 +1,7 @@ import _path from 'path'; import type { ServerRuntime as SnowpackServerRuntime } from 'snowpack'; import { fileURLToPath } from 'url'; -import type { AstroConfig, BuildOutput, GetStaticPathsResult, RouteData } from '../@types/astro'; +import type { AstroConfig, BuildOutput, RouteData } from '../@types/astro'; import { LogOptions } from '../logger'; import type { AstroRuntime } from '../runtime.js'; import { convertMatchToLocation, validateGetStaticPathsModule, validateGetStaticPathsResult } from '../util.js'; @@ -19,11 +19,13 @@ interface PageBuildOptions { /** Build dynamic page */ export async function getStaticPathsForPage({ astroConfig, + astroRuntime, snowpackRuntime, route, logging, }: { astroConfig: AstroConfig; + astroRuntime: AstroRuntime; route: RouteData; snowpackRuntime: SnowpackServerRuntime; logging: LogOptions; @@ -32,12 +34,10 @@ export async function getStaticPathsForPage({ const mod = await snowpackRuntime.importModule(location.snowpackURL); validateGetStaticPathsModule(mod); const [rssFunction, rssResult] = generateRssFunction(astroConfig.buildOptions.site, route); - const staticPaths: GetStaticPathsResult = ( - await mod.exports.getStaticPaths({ - paginate: generatePaginateFunction(route), - rss: rssFunction, - }) - ).flat(); + const staticPaths = await astroRuntime.getStaticPaths(route.component, mod, { + paginate: generatePaginateFunction(route), + rss: rssFunction, + }); validateGetStaticPathsResult(staticPaths, logging); return { paths: staticPaths.map((staticPath) => staticPath.params && route.generate(staticPath.params)).filter(Boolean), diff --git a/packages/astro/src/build/paginate.ts b/packages/astro/src/build/paginate.ts index 21a7ba94f..97a66cc2f 100644 --- a/packages/astro/src/build/paginate.ts +++ b/packages/astro/src/build/paginate.ts @@ -1,4 +1,4 @@ -import { GetStaticPathsResult, Params, Props, RouteData } from '../@types/astro'; +import { GetStaticPathsResult, PaginatedCollectionProp, PaginateFunction, Params, Props, RouteData } from '../@types/astro'; // return filters.map((filter) => { // const filteredRecipes = allRecipes.filter((recipe) => @@ -10,7 +10,7 @@ import { GetStaticPathsResult, Params, Props, RouteData } from '../@types/astro' // }); // }); -export function generatePaginateFunction(routeMatch: RouteData) { +export function generatePaginateFunction(routeMatch: RouteData): PaginateFunction { return function paginateUtility(data: any[], args: { pageSize?: number; params?: Params; props?: Props } = {}) { let { pageSize: _pageSize, params: _params, props: _props } = args; const pageSize = _pageSize || 10; @@ -54,7 +54,7 @@ export function generatePaginateFunction(routeMatch: RouteData) { next: pageNum === lastPage ? undefined : routeMatch.generate({ ...params, page: String(pageNum + 1) }), prev: pageNum === 1 ? undefined : routeMatch.generate({ ...params, page: !includesFirstPageNumber && pageNum - 1 === 1 ? undefined : String(pageNum - 1) }), }, - }, + } as PaginatedCollectionProp, }, }; }); diff --git a/packages/astro/src/build/rss.ts b/packages/astro/src/build/rss.ts index 5ba56bab6..b058970aa 100644 --- a/packages/astro/src/build/rss.ts +++ b/packages/astro/src/build/rss.ts @@ -1,4 +1,4 @@ -import type { CollectionRSS, RouteData } from '../@types/astro'; +import type { RSSFunctionArgs, RouteData } from '../@types/astro'; import parser from 'fast-xml-parser'; import { canonicalURL } from './util.js'; @@ -11,7 +11,7 @@ export function validateRSS(args: GenerateRSSArgs): void { if (!Array.isArray(rssData.items)) throw new Error(`[${srcFile}] rss.items should be an array of items`); } -type GenerateRSSArgs = { site: string; rssData: CollectionRSS; srcFile: string; feedURL: string }; +type GenerateRSSArgs = { site: string; rssData: RSSFunctionArgs; srcFile: string; feedURL: string }; /** Generate RSS 2.0 feed */ export function generateRSS(args: GenerateRSSArgs): string { diff --git a/packages/astro/src/runtime.ts b/packages/astro/src/runtime.ts index 3a63775d7..e585511d2 100644 --- a/packages/astro/src/runtime.ts +++ b/packages/astro/src/runtime.ts @@ -14,7 +14,7 @@ import { startServer as startSnowpackServer, } from 'snowpack'; import { fileURLToPath } from 'url'; -import type { AstroConfig, CollectionRSS, GetStaticPathsResult, ManifestData, Params, RuntimeMode } from './@types/astro'; +import type { AstroConfig, RSSFunctionArgs, GetStaticPathsArgs, GetStaticPathsResult, ManifestData, Params, RuntimeMode } from './@types/astro'; import { generatePaginateFunction } from './build/paginate.js'; import { canonicalURL, getSrcPath, stopTimer } from './build/util.js'; import { ConfigManager } from './config_manager.js'; @@ -27,8 +27,9 @@ import { convertMatchToLocation, validateGetStaticPathsModule, validateGetStatic const { CompileError } = parser; -interface RuntimeConfig { +export interface AstroRuntimeConfig { astroConfig: AstroConfig; + cache: { staticPaths: Record }; logging: LogOptions; mode: RuntimeMode; snowpack: SnowpackDevServer; @@ -42,7 +43,7 @@ type LoadResultSuccess = { statusCode: 200; contents: string | Buffer; contentType?: string | false; - rss?: { data: any[] & CollectionRSS }; + rss?: { data: any[] & RSSFunctionArgs }; }; type LoadResultNotFound = { statusCode: 404; error: Error }; type LoadResultError = { statusCode: 500 } & ( @@ -76,10 +77,13 @@ function getParams(array: string[]) { return fn; } -let cachedStaticPaths: Record = {}; +async function getStaticPathsMemoized(runtimeConfig: AstroRuntimeConfig, component: string, mod: any, args: GetStaticPathsArgs): Promise { + runtimeConfig.cache.staticPaths[component] = runtimeConfig.cache.staticPaths[component] || (await mod.exports.getStaticPaths(args)).flat(); + return runtimeConfig.cache.staticPaths[component]; +} /** Pass a URL to Astro to resolve and build */ -async function load(config: RuntimeConfig, rawPathname: string | undefined): Promise { +async function load(config: AstroRuntimeConfig, rawPathname: string | undefined): Promise { const { logging, snowpackRuntime, snowpack, configManager } = config; const { buildOptions, devOptions } = config.astroConfig; @@ -128,20 +132,14 @@ async function load(config: RuntimeConfig, rawPathname: string | undefined): Pro // this helps us to prevent incorrect matches in dev that wouldn't exist in build. if (!routeMatch.path) { validateGetStaticPathsModule(mod); - cachedStaticPaths[routeMatch.component] = - cachedStaticPaths[routeMatch.component] || - ( - await mod.exports.getStaticPaths({ - paginate: generatePaginateFunction(routeMatch), - rss: () => { - /* noop */ - }, - }) - ).flat(); - - validateGetStaticPathsResult(cachedStaticPaths[routeMatch.component], logging); - const routePathParams: GetStaticPathsResult = cachedStaticPaths[routeMatch.component]; - const matchedStaticPath = routePathParams.find(({ params: _params }) => JSON.stringify(_params) === JSON.stringify(params)); + const staticPaths = await getStaticPathsMemoized(config, routeMatch.component, mod, { + paginate: generatePaginateFunction(routeMatch), + rss: () => { + /* noop */ + }, + }); + validateGetStaticPathsResult(staticPaths, logging); + const matchedStaticPath = staticPaths.find(({ params: _params }) => JSON.stringify(_params) === JSON.stringify(params)); if (!matchedStaticPath) { return { statusCode: 404, error: new Error(`[getStaticPaths] route pattern matched, but no matching static path found. (${reqPath})`) }; } @@ -239,7 +237,8 @@ async function load(config: RuntimeConfig, rawPathname: string | undefined): Pro } export interface AstroRuntime { - runtimeConfig: RuntimeConfig; + runtimeConfig: AstroRuntimeConfig; + getStaticPaths: (component: string, mod: any, args: GetStaticPathsArgs) => Promise; load: (rawPathname: string | undefined) => Promise; shutdown: () => Promise; } @@ -400,8 +399,9 @@ export async function createRuntime(astroConfig: AstroConfig, { mode, logging }: snowpack = snowpackInstance; debug(logging, 'core', `snowpack created [${stopTimer(timer.backend)}]`); - const runtimeConfig: RuntimeConfig = { + const runtimeConfig: AstroRuntimeConfig = { astroConfig, + cache: { staticPaths: {} }, logging, mode, snowpack, @@ -413,7 +413,7 @@ export async function createRuntime(astroConfig: AstroConfig, { mode, logging }: snowpack.onFileChange(({ filePath }: { filePath: string }) => { // Clear out any cached getStaticPaths() data. - cachedStaticPaths = {}; + runtimeConfig.cache.staticPaths = {}; // Rebuild the manifest, if needed if (filePath.includes(fileURLToPath(astroConfig.pages))) { runtimeConfig.manifest = createManifest({ config: astroConfig }); @@ -423,6 +423,7 @@ export async function createRuntime(astroConfig: AstroConfig, { mode, logging }: return { runtimeConfig, load: load.bind(null, runtimeConfig), + getStaticPaths: getStaticPathsMemoized.bind(null, runtimeConfig), shutdown: () => snowpack.shutdown(), }; } diff --git a/packages/astro/test/astro-get-static-paths.test.js b/packages/astro/test/astro-get-static-paths.test.js new file mode 100644 index 000000000..23758168a --- /dev/null +++ b/packages/astro/test/astro-get-static-paths.test.js @@ -0,0 +1,13 @@ +import { suite } from 'uvu'; +import { setupBuild } from './helpers.js'; + +const GetStaticPaths = suite('getStaticPaths()'); + +setupBuild(GetStaticPaths, './fixtures/astro-get-static-paths'); + +GetStaticPaths('is only called once during build', async (context) => { + // It would throw if this was not true + await context.build(); +}); + +GetStaticPaths.run(); diff --git a/packages/astro/test/fixtures/astro-get-static-paths/astro.config.mjs b/packages/astro/test/fixtures/astro-get-static-paths/astro.config.mjs new file mode 100644 index 000000000..c601c31ad --- /dev/null +++ b/packages/astro/test/fixtures/astro-get-static-paths/astro.config.mjs @@ -0,0 +1,6 @@ +export default { + buildOptions: { + site: 'https://mysite.dev/blog/', + sitemap: false, + }, +}; diff --git a/packages/astro/test/fixtures/astro-get-static-paths/snowpack.config.json b/packages/astro/test/fixtures/astro-get-static-paths/snowpack.config.json new file mode 100644 index 000000000..8f034781d --- /dev/null +++ b/packages/astro/test/fixtures/astro-get-static-paths/snowpack.config.json @@ -0,0 +1,3 @@ +{ + "workspaceRoot": "../../../../../" +} diff --git a/packages/astro/test/fixtures/astro-get-static-paths/src/pages/[...test].astro b/packages/astro/test/fixtures/astro-get-static-paths/src/pages/[...test].astro new file mode 100644 index 000000000..ac763c135 --- /dev/null +++ b/packages/astro/test/fixtures/astro-get-static-paths/src/pages/[...test].astro @@ -0,0 +1,21 @@ +--- +export function getStaticPaths({paginate}) { + if (globalThis.isCalledOnce) { + throw new Error("Can only be called once!"); + } + globalThis.isCalledOnce = true; + return [ + {params: {test: 'a'}}, + {params: {test: 'b'}}, + {params: {test: 'c'}}, + ]; +} +const { params} = Astro.request; +--- + + + + Page {params.test} + + +