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] load data through regular json request #7177

Merged
merged 4 commits into from
Oct 10, 2022
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
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 @@ -196,7 +196,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';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will tree shaking remove the other stuff correctly if we write the import like this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, Rollup doesn't care either way


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 @@ -292,7 +292,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