Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[breaking] remove createIndexFiles option, derive from trailingSlash instead #3801

Merged
merged 12 commits into from
Feb 10, 2022
6 changes: 6 additions & 0 deletions .changeset/flat-stingrays-talk.md
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
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
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
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
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
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
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 — configure it via your adapter instead`
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved
),
enabled: boolean(true),
entries: validate(['*'], (input, keypath) => {
if (!Array.isArray(input) || !input.every((page) => typeof page === 'string')) {
Expand Down
Loading