From 453c026aa7f7825516e5816efc443fd9b0b6d74a Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 26 Jul 2022 08:54:49 -0400 Subject: [PATCH] Move getStaticPaths call to happen during generation (#4047) * Move getStaticPaths call to happen during generation * Adds a changeset * Update routing-priority.test.js * revert test change, clarify test purpose * Keep track of paths that have already been built Co-authored-by: Fred K. Schott --- .changeset/old-stingrays-smoke.md | 5 + packages/astro/src/core/build/generate.ts | 91 +++++++++++++++++-- packages/astro/src/core/build/page-data.ts | 63 +------------ packages/astro/src/core/build/static-build.ts | 5 +- packages/astro/src/core/build/types.ts | 1 - .../astro/test/astro-get-static-paths.test.js | 7 +- .../lit-element/src/pages/[page].astro | 30 ++++++ .../routing-priority/src/pages/de/index.astro | 6 +- packages/astro/test/lit-element.test.js | 5 + packages/astro/test/routing-priority.test.js | 18 ++-- 10 files changed, 143 insertions(+), 88 deletions(-) create mode 100644 .changeset/old-stingrays-smoke.md create mode 100644 packages/astro/test/fixtures/lit-element/src/pages/[page].astro diff --git a/.changeset/old-stingrays-smoke.md b/.changeset/old-stingrays-smoke.md new file mode 100644 index 000000000000..8e56f78ddb75 --- /dev/null +++ b/.changeset/old-stingrays-smoke.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Perf: move getStaticPaths call during the build to during page generation diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 876736e76367..e6090608f0be 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -9,8 +9,9 @@ import type { EndpointHandler, SSRLoadedRenderer, } from '../../@types/astro'; +import * as colors from 'kleur/colors'; import type { BuildInternals } from '../../core/build/internal.js'; -import { joinPaths, prependForwardSlash, removeLeadingForwardSlash } from '../../core/path.js'; +import { joinPaths, prependForwardSlash, removeLeadingForwardSlash, removeTrailingForwardSlash } from '../../core/path.js'; import type { RenderOptions } from '../../core/render/core'; import { BEFORE_HYDRATION_SCRIPT_ID } from '../../vite-plugin-scripts/index.js'; import { call as callEndpoint } from '../endpoint/index.js'; @@ -23,6 +24,8 @@ import { getOutFile, getOutFolder } from './common.js'; import { eachPageData, getPageDataByComponent } from './internal.js'; import type { PageBuildData, SingleFileBuiltModule, StaticBuildOptions } from './types'; import { getTimeStat } from './util.js'; +import { callGetStaticPaths, RouteCache, RouteCacheEntry } from '../render/route-cache.js'; +import { matchRoute } from '../routing/match.js'; // Render is usually compute, which Node.js can't parallelize well. // In real world testing, dropping from 10->1 showed a notiable perf @@ -89,10 +92,8 @@ export function chunkIsPage( } export async function generatePages( - result: RollupOutput, opts: StaticBuildOptions, internals: BuildInternals, - facadeIdToPageDataMap: Map ) { const timer = performance.now(); info(opts.logging, null, `\n${bgGreen(black(' generating static routes '))}`); @@ -102,19 +103,20 @@ export async function generatePages( const outFolder = ssr ? opts.buildConfig.server : opts.astroConfig.outDir; const ssrEntryURL = new URL('./' + serverEntry + `?time=${Date.now()}`, outFolder); const ssrEntry = await import(ssrEntryURL.toString()); + const builtPaths = new Set; for (const pageData of eachPageData(internals)) { - await generatePage(opts, internals, pageData, ssrEntry); + await generatePage(opts, internals, pageData, ssrEntry, builtPaths); } info(opts.logging, null, dim(`Completed in ${getTimeStat(timer, performance.now())}.\n`)); } async function generatePage( - //output: OutputChunk, opts: StaticBuildOptions, internals: BuildInternals, pageData: PageBuildData, - ssrEntry: SingleFileBuiltModule + ssrEntry: SingleFileBuiltModule, + builtPaths: Set, ) { let timeStart = performance.now(); const renderers = ssrEntry.renderers; @@ -148,18 +150,89 @@ async function generatePage( const icon = pageData.route.type === 'page' ? green('▶') : magenta('λ'); info(opts.logging, null, `${icon} ${pageData.route.component}`); - for (let i = 0; i < pageData.paths.length; i++) { - const path = pageData.paths[i]; + // Get paths for the route, calling getStaticPaths if needed. + const paths = await getPathsForRoute(pageData, pageModule, opts, builtPaths); + + for (let i = 0; i < paths.length; i++) { + const path = paths[i]; await generatePath(path, opts, generationOptions); const timeEnd = performance.now(); const timeChange = getTimeStat(timeStart, timeEnd); const timeIncrease = `(+${timeChange})`; const filePath = getOutputFilename(opts.astroConfig, path); - const lineIcon = i === pageData.paths.length - 1 ? '└─' : '├─'; + const lineIcon = i === paths.length - 1 ? '└─' : '├─'; info(opts.logging, null, ` ${cyan(lineIcon)} ${dim(filePath)} ${dim(timeIncrease)}`); } } +async function getPathsForRoute( + pageData: PageBuildData, + mod: ComponentInstance, + opts: StaticBuildOptions, + builtPaths: Set, +): Promise> { + let paths: Array = []; + if(pageData.route.pathname) { + paths.push(pageData.route.pathname); + builtPaths.add(pageData.route.pathname); + } else { + const route = pageData.route; + const result = await callGetStaticPaths({ + mod, + route: pageData.route, + isValidate: false, + logging: opts.logging, + ssr: opts.astroConfig.output === 'server' + }) + .then((_result) => { + const label = _result.staticPaths.length === 1 ? 'page' : 'pages'; + debug( + 'build', + `├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.magenta( + `[${_result.staticPaths.length} ${label}]` + )}` + ); + return _result; + }) + .catch((err) => { + debug('build', `├── ${colors.bold(colors.red('✗'))} ${route.component}`); + throw err; + }); + + // Save the route cache so it doesn't get called again + opts.routeCache.set(route, result); + + paths = result.staticPaths + .map((staticPath) => staticPath.params && route.generate(staticPath.params)) + .filter((staticPath) => { + // Remove empty or undefined paths + if (!staticPath) { + return false; + } + + // The path hasn't been built yet, include it + if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) { + return true; + } + + // The path was already built once. Check the manifest to see if + // this route takes priority for the final URL. + // NOTE: The same URL may match multiple routes in the manifest. + // Routing priority needs to be verified here for any duplicate + // paths to ensure routing priority rules are enforced in the final build. + const matchedRoute = matchRoute(staticPath, opts.manifest); + return matchedRoute === route; + }); + + // Add each path to the builtPaths set, to avoid building it again later. + for(const staticPath of paths) { + builtPaths.add(removeTrailingForwardSlash(staticPath)); + } + } + + return paths; +} + interface GeneratePathOptions { pageData: PageBuildData; internals: BuildInternals; diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index 74c1e45aa995..d5e63d074bae 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -1,15 +1,12 @@ import type { ViteDevServer } from 'vite'; -import type { AstroConfig, ComponentInstance, ManifestData, RouteData } from '../../@types/astro'; +import type { AstroConfig, ManifestData } from '../../@types/astro'; import type { LogOptions } from '../logger/core'; import { info } from '../logger/core.js'; import type { AllPagesData } from './types'; import * as colors from 'kleur/colors'; -import { fileURLToPath } from 'url'; import { debug } from '../logger/core.js'; -import { removeTrailingForwardSlash } from '../path.js'; -import { callGetStaticPaths, RouteCache, RouteCacheEntry } from '../render/route-cache.js'; -import { matchRoute } from '../routing/match.js'; +import { RouteCache } from '../render/route-cache.js'; export interface CollectPagesDataOptions { astroConfig: AstroConfig; @@ -30,7 +27,7 @@ export interface CollectPagesDataResult { export async function collectPagesData( opts: CollectPagesDataOptions ): Promise { - const { astroConfig, logging, manifest, origin, routeCache, viteServer } = opts; + const { astroConfig, manifest } = opts; const assets: Record = {}; const allPages: AllPagesData = {}; @@ -61,7 +58,6 @@ export async function collectPagesData( allPages[route.component] = { component: route.component, route, - paths: [route.pathname], moduleSpecifier: '', css: new Set(), hoistedScript: undefined, @@ -80,49 +76,9 @@ export async function collectPagesData( continue; } // dynamic route: - const result = await getStaticPathsForRoute(opts, route) - .then((_result) => { - const label = _result.staticPaths.length === 1 ? 'page' : 'pages'; - debug( - 'build', - `├── ${colors.bold(colors.green('✔'))} ${route.component} → ${colors.magenta( - `[${_result.staticPaths.length} ${label}]` - )}` - ); - return _result; - }) - .catch((err) => { - debug('build', `├── ${colors.bold(colors.red('✗'))} ${route.component}`); - throw err; - }); - const finalPaths = result.staticPaths - .map((staticPath) => staticPath.params && route.generate(staticPath.params)) - .filter((staticPath) => { - // Remove empty or undefined paths - if (!staticPath) { - return false; - } - - // The path hasn't been built yet, include it - if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) { - return true; - } - - // The path was already built once. Check the manifest to see if - // this route takes priority for the final URL. - // NOTE: The same URL may match multiple routes in the manifest. - // Routing priority needs to be verified here for any duplicate - // paths to ensure routing priority rules are enforced in the final build. - const matchedRoute = matchRoute(staticPath, manifest); - return matchedRoute === route; - }); - - finalPaths.map((staticPath) => builtPaths.add(removeTrailingForwardSlash(staticPath))); - allPages[route.component] = { component: route.component, route, - paths: finalPaths, moduleSpecifier: '', css: new Set(), hoistedScript: undefined, @@ -133,16 +89,3 @@ export async function collectPagesData( return { assets, allPages }; } - -async function getStaticPathsForRoute( - opts: CollectPagesDataOptions, - route: RouteData -): Promise { - const { astroConfig, logging, routeCache, ssr, viteServer } = opts; - if (!viteServer) throw new Error(`vite.createServer() not called!`); - const filePath = new URL(`./${route.component}`, astroConfig.root); - const mod = (await viteServer.ssrLoadModule(fileURLToPath(filePath))) as ComponentInstance; - const result = await callGetStaticPaths({ mod, route, isValidate: false, logging, ssr }); - routeCache.set(route, result); - return result; -} diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 110a85d677b4..99b58318d097 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -1,7 +1,6 @@ import glob from 'fast-glob'; import fs from 'fs'; import { bgGreen, bgMagenta, black, dim } from 'kleur/colors'; -import type { RollupOutput } from 'rollup'; import { fileURLToPath } from 'url'; import * as vite from 'vite'; import { BuildInternals, createBuildInternals } from '../../core/build/internal.js'; @@ -72,7 +71,7 @@ Example: // Build your project (SSR application code, assets, client JS, etc.) timer.ssr = performance.now(); info(opts.logging, 'build', `Building ${astroConfig.output} entrypoints...`); - const ssrResult = (await ssrBuild(opts, internals, pageInput)) as RollupOutput; + await ssrBuild(opts, internals, pageInput); info(opts.logging, 'build', dim(`Completed in ${getTimeStat(timer.ssr, performance.now())}.`)); const rendererClientEntrypoints = opts.astroConfig._ctx.renderers @@ -93,7 +92,7 @@ Example: timer.generate = performance.now(); if (astroConfig.output === 'static') { try { - await generatePages(ssrResult, opts, internals, facadeIdToPageDataMap); + await generatePages(opts, internals); } finally { await cleanSsrOutput(opts); } diff --git a/packages/astro/src/core/build/types.ts b/packages/astro/src/core/build/types.ts index c460e8c203be..941ec35f63d0 100644 --- a/packages/astro/src/core/build/types.ts +++ b/packages/astro/src/core/build/types.ts @@ -16,7 +16,6 @@ export type ViteID = string; export interface PageBuildData { component: ComponentPath; - paths: string[]; route: RouteData; moduleSpecifier: string; css: Set; diff --git a/packages/astro/test/astro-get-static-paths.test.js b/packages/astro/test/astro-get-static-paths.test.js index c855af04229f..ffb3660d9433 100644 --- a/packages/astro/test/astro-get-static-paths.test.js +++ b/packages/astro/test/astro-get-static-paths.test.js @@ -52,14 +52,17 @@ describe('getStaticPaths - 404 behavior', () => { }); describe('getStaticPaths - route params type validation', () => { - let fixture; - let devServer; + let fixture, devServer; before(async () => { fixture = await loadFixture({ root: './fixtures/astro-get-static-paths/' }); devServer = await fixture.startDevServer(); }); + after(async () => { + await devServer.stop(); + }); + it('resolves 200 on matching static path - string params', async () => { // route provided with { params: { year: "2022", slug: "post-2" }} const res = await fixture.fetch('/blog/2022/post-1'); diff --git a/packages/astro/test/fixtures/lit-element/src/pages/[page].astro b/packages/astro/test/fixtures/lit-element/src/pages/[page].astro new file mode 100644 index 000000000000..52a791272f22 --- /dev/null +++ b/packages/astro/test/fixtures/lit-element/src/pages/[page].astro @@ -0,0 +1,30 @@ +--- +import {MyElement} from '../components/my-element.js'; +export async function getStaticPaths() { + return [ + { + params: { page: 1 }, + }, + { + params: { page: 2 }, + }, + { + params: { page: 3 } + } + ] +}; + +const { page } = Astro.params +--- + + + + + + Posts Page {page} + + +

Welcome to page {page}

+ + + diff --git a/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro b/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro index 163ce899503c..9a82b84b988f 100644 --- a/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro +++ b/packages/astro/test/fixtures/routing-priority/src/pages/de/index.astro @@ -1,4 +1,5 @@ --- +// This route should take priority over src/pages/[lang]/index.astro --- @@ -7,6 +8,7 @@ Routing -

de/index.astro

- +

de/index.astro (priority)

+

de

+ diff --git a/packages/astro/test/lit-element.test.js b/packages/astro/test/lit-element.test.js index d314593a0f5c..9b2639cac05a 100644 --- a/packages/astro/test/lit-element.test.js +++ b/packages/astro/test/lit-element.test.js @@ -80,4 +80,9 @@ describe('LitElement test', function () { // has named slot content in lightdom expect($(namedSlot).text()).to.equal('named'); }); + + it('Is able to build when behind getStaticPaths', async () => { + const dynamicPage = await fixture.readFile('/1/index.html'); + expect(dynamicPage.length).to.be.greaterThan(0); + }); }); diff --git a/packages/astro/test/routing-priority.test.js b/packages/astro/test/routing-priority.test.js index 097ef1f8bd99..ef2daf0060a1 100644 --- a/packages/astro/test/routing-priority.test.js +++ b/packages/astro/test/routing-priority.test.js @@ -103,7 +103,7 @@ describe('Routing priority', () => { const html = await fixture.readFile('/de/index.html'); const $ = cheerioLoad(html); - expect($('h1').text()).to.equal('de/index.astro'); + expect($('h1').text()).to.equal('de/index.astro (priority)'); }); it('matches /en to [lang]/index.astro', async () => { @@ -169,28 +169,24 @@ describe('Routing priority', () => { const html = await fixture.fetch('/de').then((res) => res.text()); const $ = cheerioLoad(html); - expect($('h1').text()).to.equal('de/index.astro'); - }); - - it('matches /de to de/index.astro', async () => { - const html = await fixture.fetch('/de').then((res) => res.text()); - const $ = cheerioLoad(html); - - expect($('h1').text()).to.equal('de/index.astro'); + expect($('h1').text()).to.equal('de/index.astro (priority)'); + expect($('p').text()).to.equal('de'); }); it('matches /de/ to de/index.astro', async () => { const html = await fixture.fetch('/de/').then((res) => res.text()); const $ = cheerioLoad(html); - expect($('h1').text()).to.equal('de/index.astro'); + expect($('h1').text()).to.equal('de/index.astro (priority)'); + expect($('p').text()).to.equal('de'); }); it('matches /de/index.html to de/index.astro', async () => { const html = await fixture.fetch('/de/index.html').then((res) => res.text()); const $ = cheerioLoad(html); - expect($('h1').text()).to.equal('de/index.astro'); + expect($('h1').text()).to.equal('de/index.astro (priority)'); + expect($('p').text()).to.equal('de'); }); it('matches /en to [lang]/index.astro', async () => {