Skip to content

Commit

Permalink
[fix] load data through regular json request (#7177)
Browse files Browse the repository at this point in the history
* [fix] load data through regular json request

using devalue's stringify/parse methods
Part of #7125 and #6477

* fixes

* more idiomatic usage

* changeset

Co-authored-by: Rich Harris <[email protected]>
  • Loading branch information
dummdidumm and Rich-Harris authored Oct 10, 2022
1 parent e41d8be commit b5a0991
Show file tree
Hide file tree
Showing 15 changed files with 60 additions and 64 deletions.
7 changes: 7 additions & 0 deletions .changeset/famous-humans-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@sveltejs/adapter-netlify': patch
'@sveltejs/adapter-vercel': patch
'@sveltejs/kit': patch
---

Transfer server data as devalue-encoded JSON
2 changes: 1 addition & 1 deletion packages/adapter-netlify/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ async function generate_lambda_functions({ builder, publish, split }) {
writeFileSync(`.netlify/functions-internal/${name}.json`, fn_config);

redirects.push(`${pattern} /.netlify/functions/${name} 200`);
redirects.push(`${pattern}/__data.js /.netlify/functions/${name} 200`);
redirects.push(`${pattern}/__data.json /.netlify/functions/${name} 200`);
}
};
});
Expand Down
2 changes: 1 addition & 1 deletion packages/adapter-vercel/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export default function ({ external = [], edge, split } = {}) {
sliced_pattern = '^/?';
}

const src = `${sliced_pattern}(?:/__data.js)?$`; // TODO adding /__data.js is a temporary workaround — those endpoints should be treated as distinct routes
const src = `${sliced_pattern}(?:/__data.json)?$`; // TODO adding /__data.json is a temporary workaround — those endpoints should be treated as distinct routes

await generate_function(route.id || 'index', src, entry.generateManifest);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ export const SVELTE_KIT_ASSETS = '/_svelte_kit_assets';

export const GENERATED_COMMENT = '// this file is generated — do not edit it\n';

export const DATA_SUFFIX = '/__data.js';
export const DATA_SUFFIX = '/__data.json';
38 changes: 19 additions & 19 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { onMount, tick } from 'svelte';
import { make_trackable, decode_params, normalize_path } from '../../utils/url.js';
import { find_anchor, get_base_uri, scroll_state } from './utils.js';
import { lock_fetch, unlock_fetch, initial_fetch, subsequent_fetch } from './fetcher.js';
import {
lock_fetch,
unlock_fetch,
initial_fetch,
subsequent_fetch,
native_fetch
} from './fetcher.js';
import { parse } from './parse.js';

import Root from '__GENERATED__/root.svelte';
Expand All @@ -10,6 +16,7 @@ import { HttpError, Redirect } from '../control.js';
import { stores } from './singletons.js';
import { DATA_SUFFIX } from '../../constants.js';
import { unwrap_promises } from '../../utils/promises.js';
import * as devalue from 'devalue';

const SCROLL_KEY = 'sveltekit:scroll';
const INDEX_KEY = 'sveltekit:index';
Expand Down Expand Up @@ -1479,8 +1486,6 @@ export function create_client({ target, base, trailing_slash }) {
};
}

let data_id = 1;

/**
* @param {URL} url
* @param {boolean[]} invalid
Expand All @@ -1489,25 +1494,20 @@ let data_id = 1;
async function load_data(url, invalid) {
const data_url = new URL(url);
data_url.pathname = url.pathname.replace(/\/$/, '') + DATA_SUFFIX;
data_url.searchParams.set('__invalid', invalid.map((x) => (x ? 'y' : 'n')).join(''));
data_url.searchParams.set('__id', String(data_id++));

// The __data.js file is generated by the server and looks like
// `window.__sveltekit_data = ${devalue.uneval(data)}`. We do this instead
// of `export const data` because modules are cached indefinitely,
// and that would cause memory leaks.
//
// The data is read and deleted in the same tick as the promise
// resolves, so it's not vulnerable to race conditions
await import(/* @vite-ignore */ data_url.href);

// @ts-expect-error
const server_data = window.__sveltekit_data;
const res = await native_fetch(data_url.href, {
headers: {
'x-sveltekit-invalidated': invalid.map((x) => (x ? '1' : '')).join(',')
}
});
const server_data = await res.text();

// @ts-expect-error
delete window.__sveltekit_data;
if (!res.ok) {
// error message is a JSON-stringified string which devalue can't handle at the top level
throw new Error(JSON.parse(server_data));
}

return server_data;
return devalue.parse(server_data);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/src/runtime/client/fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { hash } from '../hash.js';

let loading = 0;

const native_fetch = window.fetch;
export const native_fetch = window.fetch;

export function lock_fetch() {
loading += 1;
Expand Down
11 changes: 3 additions & 8 deletions packages/kit/src/runtime/server/data/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { DATA_SUFFIX } from '../../../constants.js';
*/
export async function render_data(event, route, options, state) {
if (!route.page) {
// requesting /__data.js should fail for a +server.js
// requesting /__data.json should fail for a +server.js
return new Response(undefined, {
status: 404
});
Expand All @@ -25,10 +25,8 @@ export async function render_data(event, route, options, state) {
const node_ids = [...route.page.layouts, route.page.leaf];

const invalidated =
event.url.searchParams
.get('__invalid')
?.split('')
.map((x) => x === 'y') ?? node_ids.map(() => true);
event.request.headers.get('x-sveltekit-invalidated')?.split(',').map(Boolean) ??
node_ids.map(() => true);

let aborted = false;

Expand All @@ -38,9 +36,6 @@ export async function render_data(event, route, options, state) {
options.trailing_slash
);

url.searchParams.delete('__invalid');
url.searchParams.delete('__id');

const new_event = { ...event, url };

const functions = node_ids.map((n, i) => {
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 @@ -291,7 +291,7 @@ export async function respond(request, options, state) {
// add headers/cookies here, rather than inside `resolve`, so that we
// can do it once for all responses instead of once per `return`
if (!is_data_request) {
// we only want to set cookies on __data.js requests, we don't
// we only want to set cookies on __data.json requests, we don't
// want to cache stuff erroneously etc
for (const key in headers) {
const value = headers[key];
Expand Down
8 changes: 4 additions & 4 deletions packages/kit/src/runtime/server/page/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ export async function render_page(event, route, page, options, state, resolve_op

if (err instanceof Redirect) {
if (state.prerendering && should_prerender_data) {
const body = `window.__sveltekit_data = ${JSON.stringify({
const body = devalue.stringify({
type: 'redirect',
location: err.location
})}`;
});

state.prerendering.dependencies.set(data_pathname, {
response: new Response(body),
Expand Down Expand Up @@ -260,10 +260,10 @@ export async function render_page(event, route, page, options, state, resolve_op
}

if (state.prerendering && should_prerender_data) {
const body = `window.__sveltekit_data = ${devalue.uneval({
const body = devalue.stringify({
type: 'data',
nodes: branch.map((branch_node) => branch_node?.server_data)
})}`;
});

state.prerendering.dependencies.set(data_pathname, {
response: new Response(body),
Expand Down
6 changes: 3 additions & 3 deletions packages/kit/src/runtime/server/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ export function allowed_methods(mod) {
/** @param {any} data */
export function data_response(data) {
const headers = {
'content-type': 'application/javascript',
'content-type': 'application/json',
'cache-control': 'private, no-store'
};

try {
return new Response(`window.__sveltekit_data = ${devalue.uneval(data)}`, { headers });
return new Response(devalue.stringify(data), { headers });
} catch (e) {
const error = /** @type {any} */ (e);
const match = /\[(\d+)\]\.data\.(.+)/.exec(error.path);
const message = match ? `${error.message} (data.${match[2]})` : error.message;
return new Response(`throw new Error(${JSON.stringify(message)})`, { headers });
return new Response(JSON.stringify(message), { headers, status: 500 });
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/test/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ test.describe('Load', () => {
await expect(page.locator('p')).toHaveText('Count is 2');
});

test('__data.js has cache-control: private, no-store', async ({ page, clicknav }) => {
test('__data.json has cache-control: private, no-store', async ({ page, clicknav }) => {
await page.goto('/load/server-data-nostore?x=1');

const [response] = await Promise.all([
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ test.describe('Errors', () => {
);

const { status, name, message, stack, fancy } = read_errors(
'/errors/page-endpoint/get-implicit/__data.js'
'/errors/page-endpoint/get-implicit/__data.json'
);
expect(status).toBe(undefined);
expect(name).toBe('FancyError');
Expand Down
12 changes: 5 additions & 7 deletions packages/kit/test/apps/options/test/test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { parse } from 'devalue';
import { expect } from '@playwright/test';
import { start_server, test } from '../../../utils.js';

Expand Down Expand Up @@ -162,13 +163,10 @@ test.describe('trailingSlash', () => {
});

test('can fetch data from page-endpoint', async ({ request }) => {
const r = await request.get('/path-base/page-endpoint/__data.js');
const code = await r.text();
const r = await request.get('/path-base/page-endpoint/__data.json');
const data = parse(await r.text());

const window = {};
new Function('window', code)(window);

expect(window.__sveltekit_data).toEqual({
expect(data).toEqual({
type: 'data',
nodes: [null, { type: 'data', data: { message: 'hi' }, uses: {} }]
});
Expand Down Expand Up @@ -199,7 +197,7 @@ test.describe('trailingSlash', () => {
expect(requests.filter((req) => req.endsWith('.js')).length).toBeGreaterThan(0);
}

expect(requests.includes(`/path-base/prefetching/prefetched/__data.js`)).toBe(true);
expect(requests.includes(`/path-base/prefetching/prefetched/__data.json`)).toBe(true);

requests = [];
await app.goto('/path-base/prefetching/prefetched');
Expand Down
26 changes: 11 additions & 15 deletions packages/kit/test/prerendering/basics/test/test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as fs from 'fs';
import { fileURLToPath } from 'url';
import { parse } from 'devalue';
import { test } from 'uvu';
import * as assert from 'uvu/assert';

Expand All @@ -25,11 +26,9 @@ test('renders a server-side redirect', () => {
const html = read('redirect-server.html');
assert.equal(html, '<meta http-equiv="refresh" content="0;url=https://example.com/redirected">');

const code = read('redirect-server/__data.js');
const window = {};
new Function('window', code)(window);
const data = parse(read('redirect-server/__data.json'));

assert.equal(window.__sveltekit_data, {
assert.equal(data, {
type: 'redirect',
location: 'https://example.com/redirected'
});
Expand Down Expand Up @@ -77,11 +76,9 @@ test('loads a file with spaces in the filename', () => {
assert.ok(content.includes('<h1>answer: 42</h1>'), content);
});

test('generates __data.js file for shadow endpoints', () => {
const window = {};

new Function('window', read('__data.js'))(window);
assert.equal(window.__sveltekit_data, {
test('generates __data.json file for shadow endpoints', () => {
let data = parse(read('__data.json'));
assert.equal(data, {
type: 'data',
nodes: [
null,
Expand All @@ -93,8 +90,8 @@ test('generates __data.js file for shadow endpoints', () => {
]
});

new Function('window', read('shadowed-get/__data.js'))(window);
assert.equal(window.__sveltekit_data, {
data = parse(read('shadowed-get/__data.json'));
assert.equal(data, {
type: 'data',
nodes: [
null,
Expand All @@ -109,7 +106,7 @@ test('generates __data.js file for shadow endpoints', () => {

test('does not prerender page with shadow endpoint with non-load handler', () => {
assert.ok(!fs.existsSync(`${build}/shadowed-post.html`));
assert.ok(!fs.existsSync(`${build}/shadowed-post/__data.js`));
assert.ok(!fs.existsSync(`${build}/shadowed-post/__data.json`));
});

test('decodes paths when writing files', () => {
Expand Down Expand Up @@ -183,10 +180,9 @@ test('prerenders binary data', async () => {
});

test('fetches data from local endpoint', () => {
const window = {};
new Function('window', read('origin/__data.js'))(window);
const data = parse(read('origin/__data.json'));

assert.equal(window.__sveltekit_data, {
assert.equal(data, {
type: 'data',
nodes: [
null,
Expand Down
2 changes: 1 addition & 1 deletion packages/kit/test/prerendering/trailing-slash/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const read = (file) => fs.readFileSync(`${build}/${file}`, 'utf-8');
test('prerendered.paths omits trailing slashes for endpoints', () => {
const content = read('service-worker.js');

for (const path of ['/page/', '/page/__data.js', '/standalone-endpoint.json']) {
for (const path of ['/page/', '/page/__data.json', '/standalone-endpoint.json']) {
assert.ok(content.includes(`"${path}"`), `Missing ${path}`);
}
});
Expand Down

0 comments on commit b5a0991

Please sign in to comment.