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

Ensure props are loaded from matching endpoint during client-side navigation #4203

Merged
merged 4 commits into from
Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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/nasty-buttons-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

Ensure props are loaded from matching endpoint during client-side navigation
1 change: 1 addition & 0 deletions packages/kit/src/core/dev/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export async function create_plugin(config, cwd) {
if (route.type === 'page') {
return {
type: 'page',
key: route.key,
pattern: route.pattern,
params: get_params(route.params),
shadow: route.shadow
Expand Down
1 change: 1 addition & 0 deletions packages/kit/src/core/generate_manifest/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export function generate_manifest(
if (route.type === 'page') {
return `{
type: 'page',
key: ${s(route.key)},
pattern: ${route.pattern},
params: ${get_params(route.params)},
path: ${route.path ? s(route.path) : null},
Expand Down
21 changes: 10 additions & 11 deletions packages/kit/src/core/sync/create_manifest_data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,19 @@ export default function create_manifest_data({

walk(config.kit.files.routes, [], [], [], [layout], [error]);

// merge matching page/endpoint pairs into shadowed pages
const lookup = new Map();
for (const route of routes) {
if (route.type === 'page') {
lookup.set(route.key, route);
}
}

let i = routes.length;
while (i--) {
const route = routes[i];
const prev = routes[i - 1];

if (prev && prev.key === route.key) {
if (prev.type !== 'endpoint' || route.type !== 'page') {
const relative = path.relative(cwd, path.resolve(config.kit.files.routes, prev.key));
throw new Error(`Duplicate route files: ${relative}`);
}

route.shadow = prev.file;
routes.splice(--i, 1);
if (route.type === 'endpoint' && lookup.has(route.key)) {
lookup.get(route.key).shadow = route.file;
routes.splice(i, 1);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/core/sync/write_manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export function write_manifest(manifest_data, base, output) {

// optional items
if (params || route.shadow) tuple.push(params || 'null');
if (route.shadow) tuple.push('1');
if (route.shadow) tuple.push(`'${route.key}'`);

return `// ${route.a[route.a.length - 1]}\n\t\t[${tuple.join(', ')}]`;
}
Expand Down
11 changes: 8 additions & 3 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ export function create_client({ target, session, base, trailing_slash }) {
if (cached) return cached;
}

const [pattern, a, b, get_params, has_shadow] = route;
const [pattern, a, b, get_params, shadow_key] = route;
const params = get_params
? // the pattern is for the route which we've already matched to this path
get_params(/** @type {RegExpExecArray} */ (pattern.exec(path)))
Expand Down Expand Up @@ -611,18 +611,23 @@ export function create_client({ target, session, base, trailing_slash }) {
/** @type {Record<string, any>} */
let props = {};

const is_shadow_page = has_shadow && i === a.length - 1;
const is_shadow_page = shadow_key !== undefined && i === a.length - 1;

if (is_shadow_page) {
const res = await fetch(
`${url.pathname}${url.pathname.endsWith('/') ? '' : '/'}__data.json${url.search}`,
{
headers: {
'x-sveltekit-load': 'true'
'x-sveltekit-load': /** @type {string} */ (shadow_key)
}
}
);

if (res.status === 204) {
// fallthrough
return;
}

if (res.ok) {
const redirect = res.headers.get('x-sveltekit-location');

Expand Down
18 changes: 14 additions & 4 deletions packages/kit/src/runtime/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,17 @@ export async function respond(request, options, state = {}) {
event.url = new URL(event.url.origin + normalized + event.url.search);
}

// `key` will be set if this request came from a client-side navigation
// to a page with a matching endpoint
const key = request.headers.get('x-sveltekit-load');

for (const route of options.manifest._.routes) {
if (key) {
// client is requesting data for a specific endpoint
if (route.type !== 'page') continue;
if (route.key !== key) continue;
}

const match = route.pattern.exec(decoded);
if (!match) continue;

Expand All @@ -163,7 +173,7 @@ export async function respond(request, options, state = {}) {
response = await render_endpoint(event, await route.shadow());

// loading data for a client-side transition is a special case
if (request.headers.get('x-sveltekit-load') === 'true') {
if (key) {
if (response) {
// since redirects are opaque to the browser, we need to repackage
// 3xx responses as 200s with a custom header
Expand All @@ -180,9 +190,9 @@ export async function respond(request, options, state = {}) {
}
}
} else {
// TODO ideally, the client wouldn't request this data
// in the first place (at least in production)
response = new Response('{}', {
// fallthrough
response = new Response(undefined, {
status: 204,
headers: {
'content-type': 'application/json'
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/** @type {import('./[a]').RequestHandler} */
export async function get({ params }) {
const param = params.a;

if (param !== 'a') {
return {
fallthrough: true
};
}

return {
status: 200,
body: { param }
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
/** @type {string} */
export let param;
</script>

<h2>a-{param}</h2>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/** @type {import('./[b]').RequestHandler} */
export async function get({ params }) {
const param = params.b;

if (param !== 'b') {
return {
fallthrough: true
};
}

return {
status: 200,
body: { param }
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
/** @type {string} */
export let param;
</script>

<h2>b-{param}</h2>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h2>c</h2>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<a href="/shadowed/fallthrough/a">fallthrough to shadow a</a>
<a id="b" href="/shadowed/fallthrough/b">fallthrough to shadow b</a>
<a href="/shadowed/fallthrough/c">fallthrough to no shadow c</a>
12 changes: 12 additions & 0 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,18 @@ test.describe.parallel('Shadowed pages', () => {
await clicknav('[href="/shadowed/dynamic/bar"]');
expect(await page.textContent('h1')).toBe('slug: bar');
});

test('Shadow fallthrough to shadowed page', async ({ page, clicknav }) => {
await page.goto('/shadowed/fallthrough');
await clicknav('[href="/shadowed/fallthrough/b"]');
expect(await page.textContent('h2')).toBe('b-b');
});

test('Shadow fallthrough to unshadowed page', async ({ page, clicknav }) => {
await page.goto('/shadowed/fallthrough');
await clicknav('[href="/shadowed/fallthrough/c"]');
expect(await page.textContent('h2')).toBe('c');
});
});

test.describe.parallel('Endpoints', () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export type CSRComponent = any; // TODO

export type CSRComponentLoader = () => Promise<CSRComponent>;

export type CSRRoute = [RegExp, CSRComponentLoader[], CSRComponentLoader[], GetParams?, HasShadow?];
export type CSRRoute = [RegExp, CSRComponentLoader[], CSRComponentLoader[], GetParams?, ShadowKey?];

export interface EndpointData {
type: 'endpoint';
Expand All @@ -84,8 +84,6 @@ export interface EndpointData {

export type GetParams = (match: RegExpExecArray) => Record<string, string>;

type HasShadow = 1;

export interface Hooks {
externalFetch: ExternalFetch;
getSession: GetSession;
Expand Down Expand Up @@ -179,6 +177,8 @@ export interface ShadowEndpointOutput<Output extends JSONObject = JSONObject> {
body?: Output;
}

type ShadowKey = string;
Rich-Harris marked this conversation as resolved.
Show resolved Hide resolved

export interface ShadowRequestHandler<Output extends JSONObject = JSONObject> {
(event: RequestEvent): MaybePromise<Either<ShadowEndpointOutput<Output>, Fallthrough>>;
}
Expand Down Expand Up @@ -272,6 +272,7 @@ export interface SSROptions {

export interface SSRPage {
type: 'page';
key: string;
pattern: RegExp;
params: GetParams;
shadow:
Expand Down