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

Remove session #5946

Merged
merged 5 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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/happy-clouds-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[breaking] Remove session object
52 changes: 1 addition & 51 deletions documentation/docs/06-hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
title: Hooks
---

An optional `src/hooks.js` (or `src/hooks.ts`, or `src/hooks/index.js`) file exports four functions, all optional, that run on the server — `handle`, `handleError`, `getSession`, and `externalFetch`.
An optional `src/hooks.js` (or `src/hooks.ts`, or `src/hooks/index.js`) file exports three functions, all optional, that run on the server — `handle`, `handleError` and `externalFetch`.

> The location of this file can be [configured](/docs/configuration) as `config.kit.files.hooks`

Expand Down Expand Up @@ -105,56 +105,6 @@ export function handleError({ error, event }) {

> `handleError` is only called for _unexpected_ errors. It is not called for errors created with the [`error`](/docs/modules#sveltejs-kit-error) function imported from `@sveltejs/kit`, as these are _expected_ errors.

### getSession

This function takes the `event` object and returns a `session` object that is [accessible on the client](/docs/modules#$app-stores) and therefore must be safe to expose to users. It runs whenever SvelteKit server-renders a page.

If unimplemented, session is `{}`.

```js
/// file: src/hooks.js
// @filename: ambient.d.ts
declare namespace App {
interface Locals {
user: {
name: string;
email: string;
avatar: string;
token: string;
}
}
interface Session {
user?: {
name: string;
email: string;
avatar: string;
}
}
}

type MaybePromise<T> = T | Promise<T>;

// @filename: index.js
// ---cut---
/** @type {import('@sveltejs/kit').GetSession} */
export function getSession(event) {
return event.locals.user
? {
user: {
// only include properties needed client-side —
// exclude anything else attached to the user
// like access tokens etc
name: event.locals.user.name,
email: event.locals.user.email,
avatar: event.locals.user.avatar
}
}
: {};
}
```

> `session` must be serializable, which means it must not contain things like functions or custom classes, just built-in JavaScript data types

### externalFetch

This function allows you to modify (or replace) a `fetch` request for an external resource that happens inside a `load` function that runs on the server (or during pre-rendering).
Expand Down
2 changes: 1 addition & 1 deletion documentation/docs/16-types.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export type PageLoad = Kit.Load<RouteParams>;
// @errors: 2355
// ---cut---
/** @type {import('./$types').PageLoad} */
export async function load({ params, fetch, session }) {
export async function load({ params, fetch }) {
// ...
}
```
Expand Down
8 changes: 4 additions & 4 deletions documentation/docs/80-migrating.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ This file has no equivalent in SvelteKit. Any custom logic (beyond `sapper.start

#### src/server.js

When using `adapter-node` the equivalent is a [custom server](https://github.com/sveltejs/kit/tree/master/packages/adapter-node#custom-server). Otherwise, this file has no direct equivalent, since SvelteKit apps can run in serverless environments. You can, however, use the [hooks module](/docs/hooks) to implement session logic.
When using `adapter-node` the equivalent is a [custom server](https://github.com/sveltejs/kit/tree/master/packages/adapter-node#custom-server). Otherwise, this file has no direct equivalent, since SvelteKit apps can run in serverless environments.

#### src/service-worker.js

Expand Down Expand Up @@ -95,7 +95,7 @@ Any files you previously imported from directories in `src/node_modules` will ne

As before, pages and layouts can export a function that allows data to be loaded before rendering takes place.

This function has been renamed from `preload` to [`load`](/docs/load), it now lives in a `+page.js` (or `+layout.js`) next to its `+page.svelte` (or `+layout.svelte`), and its API has changed. Instead of two arguments — `page` and `session` — there is a single argument that includes both, along with `fetch` (which replaces `this.fetch`) and a new `stuff` object.
This function has been renamed from `preload` to [`load`](/docs/load), it now lives in a `+page.js` (or `+layout.js`) next to its `+page.svelte` (or `+layout.svelte`), and its API has changed. Instead of two arguments — `page` and `session` — there is a single `event` argument.

There is no more `this` object, and consequently no `this.fetch`, `this.error` or `this.redirect`. Instead of returning props directly, `load` now returns an object that _contains_ `props`, alongside various other things.

Expand All @@ -115,9 +115,9 @@ import { stores } from '@sapper/app';
const { preloading, page, session } = stores();
```

The `page` and `session` stores still exist; `preloading` has been replaced with a `navigating` store that contains `from` and `to` properties. `page` now has `url` and `params` properties, but no `path` or `query`.
The `page` store still exists; `preloading` has been replaced with a `navigating` store that contains `from` and `to` properties. `page` now has `url` and `params` properties, but no `path` or `query`.

You access them differently in SvelteKit. `stores` is now `getStores`, but in most cases it is unnecessary since you can import `navigating`, `page` and `session` directly from [`$app/stores`](/docs/modules#$app-stores).
You access them differently in SvelteKit. `stores` is now `getStores`, but in most cases it is unnecessary since you can import `navigating`, and `page` directly from [`$app/stores`](/docs/modules#$app-stores).

#### Routing

Expand Down
10 changes: 1 addition & 9 deletions packages/adapter-static/test/apps/spa/src/routes/+error.svelte
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
<script>
import { browser } from '$app/env';
import { page, session } from '$app/stores';

if (browser) {
$session.count += 1;
}
import { page } from '$app/stores';
</script>

<h1>{$page.status}</h1>
<h2>count: {$session.count}</h2>

<button on:click={() => ($session.count += 1)}>+1</button>
2 changes: 0 additions & 2 deletions packages/adapter-static/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,5 @@ run('spa', (test) => {
test('renders error page for missing page', async ({ base, page }) => {
await page.goto(`${base}/nosuchpage`);
assert.equal(await page.textContent('h1'), '404');
await page.waitForLoadState('networkidle', { timeout: 1000 });
assert.equal(await page.textContent('h2'), 'count: 1');
});
});
39 changes: 18 additions & 21 deletions packages/kit/src/runtime/app/stores.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ export const getStores = () => {
subscribe: stores.navigating.subscribe
};
},
session: stores.session,
get session() {
removed_session();
return {};
},
updated: stores.updated
};
};
Expand All @@ -54,29 +57,17 @@ export const navigating = {
}
};

/** @param {string} verb */
const throw_error = (verb) => {
function removed_session() {
// TODO remove for 1.0
throw new Error(
browser
? `Cannot ${verb} session store before subscribing`
: `Can only ${verb} session store in browser`
'stores.session is no longer available. See https://github.com/sveltejs/kit/discussions/5883'
);
};
}

/** @type {typeof import('$app/stores').session} */
export const session = {
subscribe(fn) {
const store = getStores().session;

if (browser) {
session.set = store.set;
session.update = store.update;
}

return store.subscribe(fn);
},
set: () => throw_error('set'),
update: () => throw_error('update')
subscribe: removed_session,
set: removed_session,
update: removed_session
};

/** @type {typeof import('$app/stores').updated} */
Expand All @@ -90,5 +81,11 @@ export const updated = {

return store.subscribe(fn);
},
check: () => throw_error('check')
check: () => {
throw new Error(
browser
? `Cannot check updated store before subscribing`
: `Can only check updated store in browser`
);
}
};
39 changes: 8 additions & 31 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,19 @@ function update_scroll_positions(index) {
/**
* @param {{
* target: Element;
* session: App.Session;
* base: string;
* trailing_slash: import('types').TrailingSlash;
* }} opts
* @returns {import('./types').Client}
*/
export function create_client({ target, session, base, trailing_slash }) {
export function create_client({ target, base, trailing_slash }) {
/** @type {Array<((href: string) => boolean)>} */
const invalidated = [];

const stores = {
url: notifiable_store({}),
page: notifiable_store({}),
navigating: writable(/** @type {import('types').Navigation | null} */ (null)),
session: writable(session),
updated: create_updated_store()
};

Expand Down Expand Up @@ -102,23 +100,6 @@ export function create_client({ target, session, base, trailing_slash }) {
/** @type {import('svelte').SvelteComponent} */
let root;

/** @type {App.Session} */
let $session;

let ready = false;
stores.session.subscribe(async (value) => {
$session = value;

if (!ready) return;
session_id += 1;

const current_load_uses_session = current.branch.some((node) => node?.uses.session);
if (!current_load_uses_session) return;

update(new URL(location.href), []);
});
ready = true;

let router_enabled = true;

// keeping track of the history index in order to prevent popstate navigation events if needed
Expand Down Expand Up @@ -475,7 +456,6 @@ export function create_client({ target, session, base, trailing_slash }) {
const uses = {
params: new Set(),
url: false,
session: false,
dependencies: new Set(),
parent: false
};
Expand Down Expand Up @@ -511,7 +491,6 @@ export function create_client({ target, session, base, trailing_slash }) {
});
}

const session = $session;
const load_url = new LoadURL(url);

if (node.shared?.load) {
Expand All @@ -524,10 +503,6 @@ export function create_client({ target, session, base, trailing_slash }) {
uses.url = true;
return load_url;
},
get session() {
uses.session = true;
return session;
},
async fetch(resource, init) {
let requested;

Expand Down Expand Up @@ -581,6 +556,12 @@ export function create_client({ target, session, base, trailing_slash }) {
'@migration task: Replace `props` with `data` stuff https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292693'
);
},
get session() {
// TODO remove this for 1.0
throw new Error(
'session is no longer available. See https://github.com/sveltejs/kit/discussions/5883'
);
},
get stuff() {
throw new Error(
'@migration task: Remove stuff https://github.com/sveltejs/kit/discussions/5774#discussioncomment-3292693'
Expand Down Expand Up @@ -620,8 +601,7 @@ export function create_client({ target, session, base, trailing_slash }) {

const changed = current.url && {
url: id !== current.url.pathname + current.url.search,
params: Object.keys(params).filter((key) => current.params[key] !== params[key]),
session: session_id !== current.session_id
params: Object.keys(params).filter((key) => current.params[key] !== params[key])
};

// preload modules to avoid waterfall, but handle rejections
Expand All @@ -643,7 +623,6 @@ export function create_client({ target, session, base, trailing_slash }) {
!previous ||
(changed.url && previous.uses.url) ||
changed.params.some((param) => previous.uses.params.has(param)) ||
(changed.session && previous.uses.session) ||
Array.from(previous.uses.dependencies).some((dep) => invalidated.some((fn) => fn(dep))) ||
(previous.uses.parent && nodes_changed_since_last_render.includes(true));
nodes_changed_since_last_render.push(changed_since_last_render);
Expand Down Expand Up @@ -753,7 +732,6 @@ export function create_client({ target, session, base, trailing_slash }) {
uses: {
params: new Set(),
url: false,
session: false,
dependencies: new Set(),
parent: false
}
Expand Down Expand Up @@ -825,7 +803,6 @@ export function create_client({ target, session, base, trailing_slash }) {
uses: {
params: new Set(),
url: false,
session: false,
dependencies: new Set(),
parent: false
}
Expand Down
4 changes: 1 addition & 3 deletions packages/kit/src/runtime/client/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ export { set_public_env } from '../env-public.js';
* base: string;
* },
* target: Element;
* session: any;
* route: boolean;
* spa: boolean;
* trailing_slash: import('types').TrailingSlash;
Expand All @@ -24,10 +23,9 @@ export { set_public_env } from '../env-public.js';
* };
* }} opts
*/
export async function start({ paths, target, session, route, spa, trailing_slash, hydrate }) {
export async function start({ paths, target, route, spa, trailing_slash, hydrate }) {
const client = create_client({
target,
session,
base: paths.base,
trailing_slash
});
Expand Down
1 change: 0 additions & 1 deletion packages/kit/src/runtime/client/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ export type BranchNode = {
uses: {
params: Set<string>;
url: boolean; // TODO make more granular?
session: boolean;
dependencies: Set<string>;
parent: boolean;
};
Expand Down
5 changes: 0 additions & 5 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ export async function respond(request, options, state) {
event,
options,
state,
$session: await options.hooks.getSession(event),
page_config: { router: true, hydrate: true },
status: 200,
error: null,
Expand Down Expand Up @@ -360,12 +359,10 @@ export async function respond(request, options, state) {
// if this request came direct from the user, rather than
// via a `fetch` in a `load`, render a 404 page
if (!state.initiator) {
const $session = await options.hooks.getSession(event);
return await respond_with_error({
event,
options,
state,
$session,
status: 404,
error: new Error(`Not found: ${event.url.pathname}`),
resolve_opts
Expand Down Expand Up @@ -413,12 +410,10 @@ export async function respond(request, options, state) {

// TODO is this necessary? should we just return a plain 500 at this point?
try {
const $session = await options.hooks.getSession(event);
return await respond_with_error({
event,
options,
state,
$session,
status: 500,
error,
resolve_opts
Expand Down
Loading