From 74c26843f83e86933f1de6ba6029e62dad9b0e3d Mon Sep 17 00:00:00 2001 From: Don Denton Date: Sat, 24 Jul 2021 15:38:14 -0400 Subject: [PATCH 1/2] Change config `prerender.force` to `prerender.onError` fixes sveltejs/kit#1940 - Changing from `force` to `onError` gives a somewhat more descriptive name to the option. - Using the strings "fail" and "continue" further clarifies what the option does. - Providing your own function to deal with an error allows for fine-tuning which errors fail the build and which do not. The original ticket suggested that the custom function return a boolean, but after seeing the implementation of the error handler in svelteKit, I thought it more fitting to just allow it the exact same API: throw if you want the build to fail. --- .changeset/tame-guests-collect.md | 5 +++ documentation/docs/14-configuration.md | 27 +++++++++++- packages/kit/src/core/adapt/prerender.js | 51 ++++++++++++++++------ packages/kit/src/core/config/index.spec.js | 4 +- packages/kit/src/core/config/options.js | 12 ++++- packages/kit/src/core/config/test/index.js | 2 +- packages/kit/types/config.d.ts | 11 ++++- packages/kit/types/index.d.ts | 2 +- 8 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 .changeset/tame-guests-collect.md diff --git a/.changeset/tame-guests-collect.md b/.changeset/tame-guests-collect.md new file mode 100644 index 000000000000..bac58d0d52b2 --- /dev/null +++ b/.changeset/tame-guests-collect.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Remove the `prerender.force` option in favor of `prerender.onError` diff --git a/documentation/docs/14-configuration.md b/documentation/docs/14-configuration.md index 43a45fd9a5f5..12b38ca313e4 100644 --- a/documentation/docs/14-configuration.md +++ b/documentation/docs/14-configuration.md @@ -48,7 +48,7 @@ const config = { prerender: { crawl: true, enabled: true, - force: false, + onError: 'fail', pages: ['*'] }, router: true, @@ -148,7 +148,30 @@ See [Prerendering](#ssr-and-javascript-prerender). An object containing zero or - `crawl` — determines whether SvelteKit should find pages to prerender by following links from the seed page(s) - `enabled` — set to `false` to disable prerendering altogether -- `force` — if `true`, a page that fails to render will _not_ cause the entire build to fail +- `onError` + + - `'fail'` — (default) fails the build when a routing error is encountered when following a link + - `'continue'` — allows the build to continue, despite routing errors + - `function` — custom error handler allowing you to log, `throw` and fail the build, or take other action of your choosing based on the details of the crawl + + ```ts + /** @type {import('@sveltejs/kit').PrerenderErrorHandler} */ + const handleError = ({ status, path, referrer, referenceType }) => { + if (path.startsWith('/blog')) throw new Error('Missing a blog page!'); + console.warn(`${status} ${path}${referrer ? ` (${referenceType} from ${referrer})` : ''}`); + }; + + export default { + kit: { + adapter: static(), + target: '#svelte', + prerender: { + onError: handleError + } + } + }; + ``` + - `pages` — an array of pages to prerender, or start crawling from (if `crawl: true`). The `*` string includes all non-dynamic routes (i.e. pages with no `[parameters]` ) ### router diff --git a/packages/kit/src/core/adapt/prerender.js b/packages/kit/src/core/adapt/prerender.js index 9721c91db0fa..5c06a59630c5 100644 --- a/packages/kit/src/core/adapt/prerender.js +++ b/packages/kit/src/core/adapt/prerender.js @@ -5,6 +5,12 @@ import { mkdirp } from '../filesystem/index.js'; import { __fetch_polyfill } from '../../install-fetch.js'; import { SVELTE_KIT } from '../constants.js'; +/** + * @typedef {import('types/config').PrerenderErrorHandler} PrerenderErrorHandler + * @typedef {import('types/config').PrerenderOnErrorValue} OnError + * @typedef {import('types/internal').Logger} Logger + */ + /** @param {string} html */ function clean_html(html) { return html @@ -45,13 +51,34 @@ function get_srcset_urls(attrs) { return results; } +/** @type {(errorDetails: Parameters[0] ) => string} */ +function errorDetailsToString({ status, path, referrer, referenceType }) { + return `${status} ${path}${referrer ? ` (${referenceType} from ${referrer})` : ''}`; +} + +/** @type {(log: Logger, onError: OnError) => PrerenderErrorHandler} */ +function chooseErrorHandler(log, onError) { + switch (onError) { + case 'continue': + return (errorDetails) => { + log.error(errorDetailsToString(errorDetails)); + }; + case 'fail': + return (errorDetails) => { + throw new Error(errorDetailsToString(errorDetails)); + }; + default: + return onError; + } +} + const OK = 2; const REDIRECT = 3; /** @param {{ * cwd: string; * out: string; - * log: import('types/internal').Logger; + * log: Logger; * config: import('types/config').ValidatedConfig; * build_data: import('types/internal').BuildData; * fallback?: string; @@ -75,14 +102,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a read: (file) => readFileSync(join(config.kit.files.assets, file)) }); - /** @type {(status: number, path: string, parent: string | null, verb: string) => void} */ - const error = config.kit.prerender.force - ? (status, path, parent, verb) => { - log.error(`${status} ${path}${parent ? ` (${verb} from ${parent})` : ''}`); - } - : (status, path, parent, verb) => { - throw new Error(`${status} ${path}${parent ? ` (${verb} from ${parent})` : ''}`); - }; + const error = chooseErrorHandler(log, config.kit.prerender.onError); const files = new Set([...build_data.static, ...build_data.client]); @@ -107,9 +127,9 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a /** * @param {string} path - * @param {string?} parent + * @param {string?} referrer */ - async function visit(path, parent) { + async function visit(path, referrer) { path = normalize(path); if (seen.has(path)) return; @@ -162,7 +182,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a log.info(`${rendered.status} ${path}`); writeFileSync(file, rendered.body || ''); } else if (response_type !== OK) { - error(rendered.status, path, parent, 'linked'); + error({ status: rendered.status, path, referrer, referenceType: 'linked' }); } dependencies.forEach((result, dependency_path) => { @@ -183,7 +203,12 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a if (response_type === OK) { log.info(`${result.status} ${dependency_path}`); } else { - error(result.status, dependency_path, path, 'fetched'); + error({ + status: result.status, + path: dependency_path, + referrer: path, + referenceType: 'fetched' + }); } }); diff --git a/packages/kit/src/core/config/index.spec.js b/packages/kit/src/core/config/index.spec.js index f0954248b95d..e6aea5473ec2 100644 --- a/packages/kit/src/core/config/index.spec.js +++ b/packages/kit/src/core/config/index.spec.js @@ -50,7 +50,7 @@ test('fills in defaults', () => { prerender: { crawl: true, enabled: true, - force: false, + onError: 'fail', pages: ['*'] }, router: true, @@ -150,7 +150,7 @@ test('fills in partial blanks', () => { prerender: { crawl: true, enabled: true, - force: false, + onError: 'fail', pages: ['*'] }, router: true, diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js index 633c3ad75c84..89f8b35040e2 100644 --- a/packages/kit/src/core/config/options.js +++ b/packages/kit/src/core/config/options.js @@ -121,7 +121,17 @@ const options = { children: { crawl: expect_boolean(true), enabled: expect_boolean(true), - force: expect_boolean(false), + onError: { + type: 'leaf', + default: 'fail', + validate: (option, keypath) => { + if (typeof option === 'function') return option; + if (['continue', 'fail'].includes(option)) return option; + throw new Error( + `${keypath} should be either a custom function or one of "continue" or "fail"` + ); + } + }, pages: { type: 'leaf', default: ['*'], diff --git a/packages/kit/src/core/config/test/index.js b/packages/kit/src/core/config/test/index.js index daa1c8b400b7..84d5fc784a49 100644 --- a/packages/kit/src/core/config/test/index.js +++ b/packages/kit/src/core/config/test/index.js @@ -54,7 +54,7 @@ async function testLoadDefaultConfig(path) { exclude: [] }, paths: { base: '', assets: '/.' }, - prerender: { crawl: true, enabled: true, force: false, pages: ['*'] }, + prerender: { crawl: true, enabled: true, onError: 'fail', pages: ['*'] }, router: true, ssr: true, target: null, diff --git a/packages/kit/types/config.d.ts b/packages/kit/types/config.d.ts index 2e11ce54670d..4f97b93d6ef0 100644 --- a/packages/kit/types/config.d.ts +++ b/packages/kit/types/config.d.ts @@ -78,6 +78,15 @@ export interface Config { preprocess?: any; } +export type PrerenderErrorHandler = (errorDetails: { + status: number; + path: string; + referrer: string | null; + referenceType: 'linked' | 'fetched'; +}) => void | never; + +export type PrerenderOnErrorValue = 'fail' | 'continue' | PrerenderErrorHandler; + export interface ValidatedConfig { compilerOptions: any; extensions: string[]; @@ -117,7 +126,7 @@ export interface ValidatedConfig { prerender: { crawl: boolean; enabled: boolean; - force: boolean; + onError: PrerenderOnErrorValue; pages: string[]; }; router: boolean; diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index be0495bd1efb..683031a2dc78 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -3,7 +3,7 @@ import './ambient-modules'; -export { Adapter, AdapterUtils, Config, ValidatedConfig } from './config'; +export { Adapter, AdapterUtils, Config, ValidatedConfig, PrerenderErrorHandler } from './config'; export { EndpointOutput, RequestHandler } from './endpoint'; export { ErrorLoad, ErrorLoadInput, Load, LoadInput, LoadOutput, Page } from './page'; export { From c6f5a107ef49d0200e9cc686342fcf4ee5451ad3 Mon Sep 17 00:00:00 2001 From: Don Denton Date: Sat, 24 Jul 2021 21:14:17 -0400 Subject: [PATCH 2/2] Add failure message for use of `prerender.force` This commit can be reverted for the 1.0 release. It is purely here for ease of transition from `force` to `onError`. --- packages/kit/src/core/config/index.spec.js | 4 ++++ packages/kit/src/core/config/options.js | 16 ++++++++++++++++ packages/kit/src/core/config/test/index.js | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/core/config/index.spec.js b/packages/kit/src/core/config/index.spec.js index e6aea5473ec2..efddafd97217 100644 --- a/packages/kit/src/core/config/index.spec.js +++ b/packages/kit/src/core/config/index.spec.js @@ -50,6 +50,8 @@ test('fills in defaults', () => { prerender: { crawl: true, enabled: true, + // TODO: remove this for the 1.0 release + force: undefined, onError: 'fail', pages: ['*'] }, @@ -150,6 +152,8 @@ test('fills in partial blanks', () => { prerender: { crawl: true, enabled: true, + // TODO: remove this for the 1.0 release + force: undefined, onError: 'fail', pages: ['*'] }, diff --git a/packages/kit/src/core/config/options.js b/packages/kit/src/core/config/options.js index 89f8b35040e2..68f688a4c0fb 100644 --- a/packages/kit/src/core/config/options.js +++ b/packages/kit/src/core/config/options.js @@ -121,6 +121,22 @@ const options = { children: { crawl: expect_boolean(true), enabled: expect_boolean(true), + // TODO: remove this for the 1.0 release + force: { + type: 'leaf', + default: undefined, + validate: (option, keypath) => { + if (typeof option !== undefined) { + const newSetting = option ? 'continue' : 'fail'; + const needsSetting = newSetting === 'continue'; + throw new Error( + `${keypath} has been removed in favor of \`onError\`. In your case, set \`onError\` to "${newSetting}"${ + needsSetting ? '' : ' (or leave it undefined)' + } to get the same behavior as you would with \`force: ${JSON.stringify(option)}\`` + ); + } + } + }, onError: { type: 'leaf', default: 'fail', diff --git a/packages/kit/src/core/config/test/index.js b/packages/kit/src/core/config/test/index.js index 84d5fc784a49..c378835273ad 100644 --- a/packages/kit/src/core/config/test/index.js +++ b/packages/kit/src/core/config/test/index.js @@ -54,7 +54,7 @@ async function testLoadDefaultConfig(path) { exclude: [] }, paths: { base: '', assets: '/.' }, - prerender: { crawl: true, enabled: true, onError: 'fail', pages: ['*'] }, + prerender: { crawl: true, enabled: true, force: undefined, onError: 'fail', pages: ['*'] }, router: true, ssr: true, target: null,