Skip to content

Commit

Permalink
Ensure props are loaded from matching endpoint during client-side nav…
Browse files Browse the repository at this point in the history
…igation (#4203)

* add failing test from #4139

* fix #4038

* changeset

* add comment
  • Loading branch information
Rich-Harris authored Mar 4, 2022
1 parent c8b8d76 commit 4c5322a
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 22 deletions.
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 @@ -69,6 +69,7 @@ export function generate_manifest({ build_data, relative_path, routes, format =
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 @@ -266,20 +266,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 @@ -567,7 +567,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 @@ -618,18 +618,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
12 changes: 9 additions & 3 deletions packages/kit/types/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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 @@ -83,8 +83,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 @@ -178,6 +176,13 @@ export interface ShadowEndpointOutput<Output extends JSONObject = JSONObject> {
body?: Output;
}

/**
* The route key of a page with a matching endpoint — used to ensure the
* client loads data from the right endpoint during client-side navigation
* rather than a different route that happens to match the path
*/
type ShadowKey = string;

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

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

0 comments on commit 4c5322a

Please sign in to comment.