Skip to content

Commit

Permalink
[breaking] remove createIndexFiles option, derive from `trailingSla…
Browse files Browse the repository at this point in the history 
…sh` instead (#3801)

* move createIndexFiles option to adapter-static

* dont provide default value here

* add missing tests

* derive createIndexFiles from trailingSlash

* robustify prerendering with paths.base

* update tests

* update changeset

* remove internal createIndexFiles option

* update docs

* fix test

* Update packages/kit/src/core/config/options.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
  • Loading branch information
@Rich-Harris @benmccann
Rich-Harris and benmccann committed Feb 10, 2022
1 parent dac52a7 commit c9e9809
Show file tree
Hide file tree
Showing 11 changed files with 114 additions and 106 deletions.
6 changes: 6 additions & 0 deletions .changeset/flat-stingrays-talk.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sveltejs/adapter-static': patch
'@sveltejs/kit': patch
---

[breaking] remove `createIndexFiles` option, derive from `trailingSlash` instead
4 changes: 2 additions & 2 deletions documentation/docs/14-configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ const config = {
prerender: {
concurrency: 1,
crawl: true,
createIndexFiles: true,
enabled: true,
entries: ['*'],
onError: 'fail'
Expand Down Expand Up @@ -203,7 +202,6 @@ See [Prerendering](/docs/page-options#prerender). An object containing zero or m

- `concurrency` — how many pages can be prerendered simultaneously. JS is single-threaded, but in cases where prerendering performance is network-bound (for example loading content from a remote CMS) this can speed things up by processing other tasks while waiting on the network response
- `crawl` — determines whether SvelteKit should find pages to prerender by following links from the seed page(s)
- `createIndexFiles` - if set to `false`, will render `about.html` instead of `about/index.html`
- `enabled` — set to `false` to disable prerendering altogether
- `entries` — 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]` )
- `onError`
Expand Down Expand Up @@ -249,6 +247,8 @@ Whether to remove, append, or ignore trailing slashes when resolving URLs to rou
- `"always"` — redirect `/x` to `/x/`
- `"ignore"` — don't automatically add or remove trailing slashes. `/x` and `/x/` will be treated equivalently

This option also affects [prerendering](/docs/page-options#prerender). If `trailingSlash` is `always`, a route like `/about` will result in an `about/index.html` file, otherwise it will create `about.html`, mirroring static webserver conventions.

> Ignoring trailing slashes is not recommended — the semantics of relative paths differ between the two cases (`./y` from `/x` is `/y`, but from `/x/` is `/x/y`), and `/x` and `/x/` are treated as separate URLs which is harmful to SEO. If you use this option, ensure that you implement logic for conditionally adding or removing trailing slashes from `request.path` inside your [`handle`](/docs/hooks#handle) function.
### version
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-static/test/test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ run('spa', (test) => {
});

test('prerenders page with prerender=true', ({ cwd }) => {
assert.ok(fs.existsSync(`${cwd}/build/about/index.html`));
assert.ok(fs.existsSync(`${cwd}/build/about.html`));
});

test('renders content in fallback page when JS runs', async ({ base, page }) => {
Expand Down
3 changes: 2 additions & 1 deletion packages/kit/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@
"prepublishOnly": "npm run build",
"test": "npm run test:unit && npm run test:packaging && npm run test:prerendering && npm run test:integration",
"test:unit": "uvu src \"(spec\\.js|test[\\\\/]index\\.js)\" -i packaging",
"test:prerendering": "pnpm test:prerendering:basics",
"test:prerendering": "pnpm test:prerendering:basics && pnpm test:prerendering:options",
"test:prerendering:basics": "cd test/prerendering/basics && pnpm test",
"test:prerendering:options": "cd test/prerendering/options && pnpm test",
"test:packaging": "uvu src/packaging \"(spec\\.js|test[\\\\/]index\\.js)\"",
"test:integration": "pnpm test:integration:amp && pnpm test:integration:basics && pnpm test:integration:options && pnpm test:integration:options-2",
"test:integration:amp": "cd test/apps/amp && pnpm test",
Expand Down
176 changes: 89 additions & 87 deletions packages/kit/src/core/adapt/prerender/prerender.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,21 @@ import { escape_html_attr } from '../../../utils/escape.js';
* @typedef {import('types/internal').Logger} Logger
*/

/** @type {(errorDetails: Parameters<PrerenderErrorHandler>[0] ) => string} */
function errorDetailsToString({ status, path, referrer, referenceType }) {
/** @type {(details: Parameters<PrerenderErrorHandler>[0] ) => string} */
function format_error({ status, path, referrer, referenceType }) {
return `${status} ${path}${referrer ? ` (${referenceType} from ${referrer})` : ''}`;
}

/** @type {(log: Logger, onError: OnError) => PrerenderErrorHandler} */
function chooseErrorHandler(log, onError) {
function normalise_error_handler(log, onError) {
switch (onError) {
case 'continue':
return (errorDetails) => {
log.error(errorDetailsToString(errorDetails));
return (details) => {
log.error(format_error(details));
};
case 'fail':
return (errorDetails) => {
throw new Error(errorDetailsToString(errorDetails));
return (details) => {
throw new Error(format_error(details));
};
default:
return onError;
Expand Down Expand Up @@ -79,7 +79,7 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a

const app = new App(manifest);

const error = chooseErrorHandler(log, config.kit.prerender.onError);
const error = normalise_error_handler(log, config.kit.prerender.onError);

const files = new Set([
...build_data.static,
Expand Down Expand Up @@ -116,12 +116,15 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
* @param {boolean} is_html
*/
function output_filename(path, is_html) {
path = path.slice(config.kit.paths.base.length) || '/';

if (path === '/') {
return '/index.html';
}

const parts = path.split('/');
if (is_html && parts[parts.length - 1] !== 'index.html') {
if (config.kit.prerender.createIndexFiles) {
if (config.kit.trailingSlash === 'always') {
parts.push('index.html');
} else {
parts[parts.length - 1] += '.html';
Expand Down Expand Up @@ -149,116 +152,115 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
* @param {string?} referrer
*/
async function visit(path, decoded_path, referrer) {
if (!path.startsWith(config.kit.paths.base)) {
error({ status: 404, path, referrer, referenceType: 'linked' });
return;
}

/** @type {Map<string, import('types/internal').PrerenderDependency>} */
const dependencies = new Map();

const render_path = config.kit.paths?.base
? `http://sveltekit-prerender${config.kit.paths.base}${path === '/' ? '' : path}`
: `http://sveltekit-prerender${path}`;

const rendered = await app.render(new Request(render_path), {
const rendered = await app.render(new Request(`http://sveltekit-prerender${path}`), {
prerender: {
all,
dependencies
}
});

if (rendered) {
const response_type = Math.floor(rendered.status / 100);
const type = rendered.headers.get('content-type');
const is_html = response_type === REDIRECT || type === 'text/html';
const response_type = Math.floor(rendered.status / 100);
const type = rendered.headers.get('content-type');
const is_html = response_type === REDIRECT || type === 'text/html';

const file = `${out}${output_filename(decoded_path, is_html)}`;
const file = `${out}${output_filename(decoded_path, is_html)}`;

if (response_type === REDIRECT) {
const location = rendered.headers.get('location');
if (response_type === REDIRECT) {
const location = rendered.headers.get('location');

if (location) {
mkdirp(dirname(file));
if (location) {
mkdirp(dirname(file));

log.warn(`${rendered.status} ${decoded_path} -> ${location}`);
log.warn(`${rendered.status} ${decoded_path} -> ${location}`);

writeFileSync(
file,
`<meta http-equiv="refresh" content=${escape_html_attr(`0;url=${location}`)}>`
);
writeFileSync(
file,
`<meta http-equiv="refresh" content=${escape_html_attr(`0;url=${location}`)}>`
);

const resolved = resolve(path, location);
if (is_root_relative(resolved)) {
enqueue(resolved, path);
}
} else {
log.warn(`location header missing on redirect received from ${decoded_path}`);
const resolved = resolve(path, location);
if (is_root_relative(resolved)) {
enqueue(resolved, path);
}

return;
} else {
log.warn(`location header missing on redirect received from ${decoded_path}`);
}

const text = await rendered.text();
return;
}

if (rendered.status === 200) {
mkdirp(dirname(file));
const text = await rendered.text();

log.info(`${rendered.status} ${decoded_path}`);
writeFileSync(file, text);
paths.push(normalize(decoded_path));
} else if (response_type !== OK) {
error({ status: rendered.status, path, referrer, referenceType: 'linked' });
}
if (rendered.status === 200) {
mkdirp(dirname(file));

for (const [dependency_path, result] of dependencies) {
const { status, headers } = result.response;
log.info(`${rendered.status} ${decoded_path}`);
writeFileSync(file, text);
paths.push(normalize(decoded_path));
} else if (response_type !== OK) {
error({ status: rendered.status, path, referrer, referenceType: 'linked' });
}

const response_type = Math.floor(status / 100);
for (const [dependency_path, result] of dependencies) {
const { status, headers } = result.response;

const is_html = headers.get('content-type') === 'text/html';
const response_type = Math.floor(status / 100);

const file = `${out}${output_filename(dependency_path, is_html)}`;
mkdirp(dirname(file));
const is_html = headers.get('content-type') === 'text/html';

writeFileSync(
file,
result.body === null ? new Uint8Array(await result.response.arrayBuffer()) : result.body
);
paths.push(dependency_path);

if (response_type === OK) {
log.info(`${status} ${dependency_path}`);
} else {
error({
status,
path: dependency_path,
referrer: path,
referenceType: 'fetched'
});
}
}
const file = `${out}${output_filename(dependency_path, is_html)}`;
mkdirp(dirname(file));

if (is_html && config.kit.prerender.crawl) {
for (const href of crawl(text)) {
if (href.startsWith('data:') || href.startsWith('#')) continue;
writeFileSync(
file,
result.body === null ? new Uint8Array(await result.response.arrayBuffer()) : result.body
);
paths.push(dependency_path);

const resolved = resolve(path, href);
if (!is_root_relative(resolved)) continue;
if (response_type === OK) {
log.info(`${status} ${dependency_path}`);
} else {
error({
status,
path: dependency_path,
referrer: path,
referenceType: 'fetched'
});
}
}

const parsed = new URL(resolved, 'http://localhost');
if (is_html && config.kit.prerender.crawl) {
for (const href of crawl(text)) {
if (href.startsWith('data:') || href.startsWith('#')) continue;

let pathname = decodeURI(parsed.pathname);
const resolved = resolve(path, href);
if (!is_root_relative(resolved)) continue;

if (config.kit.paths.base) {
if (!pathname.startsWith(config.kit.paths.base)) continue;
pathname = pathname.slice(config.kit.paths.base.length) || '/';
}
const parsed = new URL(resolved, 'http://localhost');

const file = pathname.slice(1);
if (files.has(file)) continue;
let pathname = decodeURI(parsed.pathname);

if (parsed.search) {
// TODO warn that query strings have no effect on statically-exported pages
}
if (config.kit.paths.base) {
if (!pathname.startsWith(config.kit.paths.base)) continue;
pathname = pathname.slice(config.kit.paths.base.length) || '/';
}

const file = pathname.slice(1);
if (files.has(file)) continue;

enqueue(pathname, path);
if (parsed.search) {
// TODO warn that query strings have no effect on statically-exported pages
}

enqueue(pathname, path);
}
}
}
Expand All @@ -267,10 +269,10 @@ export async function prerender({ cwd, out, log, config, build_data, fallback, a
for (const entry of config.kit.prerender.entries) {
if (entry === '*') {
for (const entry of build_data.entries) {
enqueue(entry, null);
enqueue(config.kit.paths.base + entry, null);
}
} else {
enqueue(entry, null);
enqueue(config.kit.paths.base + entry, null);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/core/config/index.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const get_defaults = (prefix = '') => ({
prerender: {
concurrency: 1,
crawl: true,
createIndexFiles: true,
createIndexFiles: undefined,
enabled: true,
entries: ['*'],
force: undefined,
Expand Down
4 changes: 3 additions & 1 deletion packages/kit/src/core/config/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,9 @@ const options = object(
prerender: object({
concurrency: number(1),
crawl: boolean(true),
createIndexFiles: boolean(true),
createIndexFiles: error(
(keypath) => `${keypath} has been removed — it is now controlled by the trailingSlash option. See https://kit.svelte.dev/docs/configuration#trailingslash`
),
enabled: boolean(true),
entries: validate(['*'], (input, keypath) => {
if (!Array.isArray(input) || !input.every((page) => typeof page === 'string')) {
Expand Down
Loading

0 comments on commit c9e9809

Please sign in to comment.