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

[fix] revert change to rendering options (#2008) #2047

Merged
merged 3 commits into from
Aug 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chilled-bees-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] revert change to rendering options (#2008)
10 changes: 4 additions & 6 deletions documentation/docs/11-ssr-and-javascript.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@ By default, SvelteKit will render any component first on the server and send it

You can control each of these on a per-app or per-page basis. Note that each of the per-page settings use [`context="module"`](https://svelte.dev/docs#script_context_module), and only apply to page components, _not_ [layout](#layouts) components.

The app-wide config options take a function, which lets you set configure the option in an advanced manner on a per-request and per-page basis. E.g. you could disable SSR for `/admin` or enable SSR only for search engine crawlers (aka [dynamic rendering](https://developers.google.com/search/docs/advanced/javascript/dynamic-rendering)).

Each setting can be controlled independently, but `ssr` and `hydrate` cannot both be `false` since that would result in nothing being rendered at all.
If both are specified, per-page settings override per-app settings in case of conflicts. Each setting can be controlled independently, but `ssr` and `hydrate` cannot both be `false` since that would result in nothing being rendered at all.

### ssr

Disabling [server-side rendering](#appendix-ssr) effectively turns your SvelteKit app into a [**single-page app** or SPA](#appendix-csr-and-spa). The default app-wide config option value is a function which reads the page value. Reading the page value causes the page to be loaded on the server. If you'd like to avoid this because you're building a SPA, you will need to set a value such as a boolean for each of the four rendering options which does not access the page-level settings.
Disabling [server-side rendering](#appendix-ssr) effectively turns your SvelteKit app into a [**single-page app** or SPA](#appendix-csr-and-spa).

> In most situations this is not recommended: see [the discussion in the appendix](#appendix-ssr). Consider whether it's truly appropriate to disable and don't simply disable SSR because you've hit an issue with it.

SSR can be configured with app-wide [`ssr` config option](#configuration-ssr), or a page-level `ssr` export:
You can disable SSR app-wide with the [`ssr` config option](#configuration-ssr), or a page-level `ssr` export:

```html
<script context="module">
Expand All @@ -28,7 +26,7 @@ SSR can be configured with app-wide [`ssr` config option](#configuration-ssr), o

SvelteKit includes a [client-side router](#appendix-routing) that intercepts navigations (from the user clicking on links, or interacting with the back/forward buttons) and updates the page contents, rather than letting the browser handle the navigation by reloading.

In certain circumstances you might need to configure [client-side routing](#appendix-routing) with the app-wide [`router` config option](#configuration-router) or the page-level `router` export:
In certain circumstances you might need to disable [client-side routing](#appendix-routing) with the app-wide [`router` config option](#configuration-router) or the page-level `router` export:

```html
<script context="module">
Expand Down
21 changes: 4 additions & 17 deletions documentation/docs/14-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ const config = {
floc: false,
host: null,
hostHeader: null,
// hydrate unless disabled on page
hydrate: async ({ page }) => {
const leaf = await page;
return 'hydrate' in leaf ? !!leaf.hydrate : true;
},
hydrate: true,
package: {
dir: 'package',
emitTypes: true,
Expand All @@ -51,24 +47,15 @@ const config = {
},
prerender: {
crawl: true,
// don't prerender unless enabled on page
enabled: expect_page_scriptable(async ({ page }) => !!(await page).prerender),
enabled: true,
onError: 'fail',
pages: ['*']
},
// route unless disabled on page
router: async ({ page }) => {
const leaf = await page;
return 'router' in leaf ? !!leaf.router : true;
},
router: true,
serviceWorker: {
exclude: []
},
// do SSR unless disabled on page
ssr: async ({ page }) => {
const leaf = await page;
return 'ssr' in leaf ? !!leaf.ssr : true;
},
ssr: true,
target: null,
trailingSlash: 'never',
vite: () => ({})
Expand Down
13 changes: 8 additions & 5 deletions documentation/faq/80-integrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Please use SDK v9 which provides a modular SDK approach that's currently in beta

### How do I use a client-side only library that depends on `document` or `window`?

Vite will attempt to process all imported libraries and may fail when encountering a library that is not compatible with SSR. This occurs even when SSR is disabled unless you also override the default values for `hydrate`, `router`, and `prerender.enabled` [as mentioned in the docs](docs#ssr-and-javascript-ssr).
Vite will attempt to process all imported libraries and may fail when encountering a library that is not compatible with SSR. [This currently occurs even when SSR is disabled](https://github.com/sveltejs/kit/issues/754).

If you need access to the `document` or `window` variables or otherwise need it to run only on the client-side you can wrap it in a `browser` check:

Expand All @@ -30,17 +30,20 @@ if (browser) {
}
```

You can also run code in `onMount` if you'd like to run it after the component has been first rendered to the DOM.
You can also run code in `onMount` if you'd like to run it after the component has been first rendered to the DOM. In this case, you may still find a benefit of including a `browser` check as shown below because Vite may otherwise attempt to optimize the dependency and fail on it. [We hope to make this unnecessary in the future](https://github.com/sveltejs/svelte/issues/6372).

```html
<script>
import { onMount } from 'svelte';
import { browser } from '$app/env';

let lib;

onMount(async () => {
lib = (await import('some-browser-only-library')).default;
});
if (browser) {
onMount(async () => {
lib = (await import('some-browser-only-library')).default;
});
}
</script>
```

Expand Down
7 changes: 3 additions & 4 deletions packages/kit/src/core/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,17 +335,16 @@ async function build_server(
error.stack = options.get_stack(error);
},
hooks: get_hooks(user_hooks),
hydrate: ${config.kit.hydrate},
hydrate: ${s(config.kit.hydrate)},
initiator: undefined,
load_component,
manifest,
paths: settings.paths,
prerender: ${config.kit.prerender && config.kit.prerender.enabled},
read: settings.read,
root,
service_worker: ${service_worker_entry_file ? "'/service-worker.js'" : 'null'},
router: ${config.kit.router},
ssr: ${config.kit.ssr},
router: ${s(config.kit.router)},
ssr: ${s(config.kit.ssr)},
target: ${s(config.kit.target)},
template,
trailing_slash: ${s(config.kit.trailingSlash)}
Expand Down
29 changes: 13 additions & 16 deletions packages/kit/src/core/config/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import { deep_merge, validate_config } from './index.js';

test('fills in defaults', () => {
const validated = validate_config({});
delete_complex_opts(validated);

// @ts-expect-error
delete validated.kit.vite;

assert.equal(validated, {
compilerOptions: null,
Expand All @@ -25,6 +27,7 @@ test('fills in defaults', () => {
floc: false,
host: null,
hostHeader: null,
hydrate: true,
package: {
dir: 'package',
exports: {
Expand All @@ -46,11 +49,14 @@ test('fills in defaults', () => {
},
prerender: {
crawl: true,
enabled: true,
// TODO: remove this for the 1.0 release
force: undefined,
onError: 'fail',
pages: ['*']
},
router: true,
ssr: true,
target: null,
trailingSlash: 'never'
},
Expand Down Expand Up @@ -101,7 +107,8 @@ test('fills in partial blanks', () => {

assert.equal(validated.kit.vite(), {});

delete_complex_opts(validated);
// @ts-expect-error
delete validated.kit.vite;

assert.equal(validated, {
compilerOptions: null,
Expand All @@ -122,6 +129,7 @@ test('fills in partial blanks', () => {
floc: false,
host: null,
hostHeader: null,
hydrate: true,
package: {
dir: 'package',
exports: {
Expand All @@ -143,11 +151,14 @@ test('fills in partial blanks', () => {
},
prerender: {
crawl: true,
enabled: true,
// TODO: remove this for the 1.0 release
force: undefined,
onError: 'fail',
pages: ['*']
},
router: true,
ssr: true,
target: null,
trailingSlash: 'never'
},
Expand Down Expand Up @@ -506,17 +517,3 @@ deepMergeSuite('merge including toString', () => {
});

deepMergeSuite.run();

/** @param {import('types/config').ValidatedConfig} validated */
function delete_complex_opts(validated) {
// @ts-expect-error
delete validated.kit.vite;
// @ts-expect-error
delete validated.kit.hydrate;
// @ts-expect-error
delete validated.kit.prerender.enabled;
// @ts-expect-error
delete validated.kit.router;
// @ts-expect-error
delete validated.kit.ssr;
}
34 changes: 4 additions & 30 deletions packages/kit/src/core/config/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ const options = {

hostHeader: expect_string(null),

hydrate: expect_page_scriptable(async ({ page }) => {
const leaf = await page;
return 'hydrate' in leaf ? !!leaf.hydrate : true;
}),
hydrate: expect_boolean(true),
serviceWorker: {
type: 'branch',
children: {
Expand Down Expand Up @@ -123,7 +120,7 @@ const options = {
type: 'branch',
children: {
crawl: expect_boolean(true),
enabled: expect_page_scriptable(async ({ page }) => !!(await page).prerender),
enabled: expect_boolean(true),
// TODO: remove this for the 1.0 release
force: {
type: 'leaf',
Expand Down Expand Up @@ -173,15 +170,9 @@ const options = {
}
},

router: expect_page_scriptable(async ({ page }) => {
const leaf = await page;
return 'router' in leaf ? !!leaf.router : true;
}),
router: expect_boolean(true),

ssr: expect_page_scriptable(async ({ page }) => {
const leaf = await page;
return 'ssr' in leaf ? !!leaf.ssr : true;
}),
ssr: expect_boolean(true),

target: expect_string(null),

Expand Down Expand Up @@ -268,23 +259,6 @@ function expect_boolean(boolean) {
};
}

/**
* @param {import('types/config').ScriptablePageOpt<boolean>} value
* @returns {ConfigDefinition}
*/
function expect_page_scriptable(value) {
return {
type: 'leaf',
default: value,
validate: (option, keypath) => {
if (typeof option !== 'boolean' && typeof option !== 'function') {
throw new Error(`${keypath} should be a boolean or function that returns one`);
}
return option;
}
};
}

/**
* @param {string[]} options
* @returns {ConfigDefinition}
Expand Down
23 changes: 7 additions & 16 deletions packages/kit/src/core/config/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ async function testLoadDefaultConfig(path) {
const cwd = join(__dirname, 'fixtures', path);

const config = await load_config({ cwd });
delete_complex_opts(config);

// @ts-expect-error
delete config.kit.vite; // can't test equality of a function

assert.equal(config, {
compilerOptions: null,
Expand All @@ -35,6 +37,7 @@ async function testLoadDefaultConfig(path) {
floc: false,
host: null,
hostHeader: null,
hydrate: true,
package: {
dir: 'package',
exports: {
Expand All @@ -51,7 +54,9 @@ async function testLoadDefaultConfig(path) {
exclude: []
},
paths: { base: '', assets: '/.' },
prerender: { crawl: true, force: undefined, onError: 'fail', pages: ['*'] },
prerender: { crawl: true, enabled: true, force: undefined, onError: 'fail', pages: ['*'] },
router: true,
ssr: true,
target: null,
trailingSlash: 'never'
},
Expand Down Expand Up @@ -98,17 +103,3 @@ test('errors on loading config with incorrect default export', async () => {
});

test.run();

/** @param {import('types/config').ValidatedConfig} validated */
function delete_complex_opts(validated) {
// @ts-expect-error
delete validated.kit.vite;
// @ts-expect-error
delete validated.kit.hydrate;
// @ts-expect-error
delete validated.kit.prerender.enabled;
// @ts-expect-error
delete validated.kit.router;
// @ts-expect-error
delete validated.kit.ssr;
}
1 change: 0 additions & 1 deletion packages/kit/src/core/dev/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,6 @@ async function create_handler(vite, config, dir, cwd, manifest) {
};
},
manifest,
prerender: config.kit.prerender.enabled,
read: (file) => fs.readFileSync(path.join(config.kit.files.assets, file)),
root,
router: config.kit.router,
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export async function respond(incoming, options, state = {}) {
return await render_response({
options,
$session: await options.hooks.getSession(request),
page_config: { ssr: false, router: true, hydrate: true, prerender: true },
page_config: { ssr: false, router: true, hydrate: true },
status: 200,
branch: []
});
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/server/page/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const s = JSON.stringify;
* @param {{
* options: import('types/internal').SSRRenderOptions;
* $session: any;
* page_config: import('types/config').PageOpts;
* page_config: { hydrate: boolean, router: boolean, ssr: boolean };
* status: number;
* error?: Error,
* branch?: Array<import('./types').Loaded>;
Expand Down
Loading