Skip to content

[fix] better type generation for load functions with different return values #7425

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

Merged
merged 3 commits into from
Oct 28, 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
5 changes: 5 additions & 0 deletions .changeset/friendly-pears-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

[fix] better type generation for load functions with different return values
9 changes: 7 additions & 2 deletions packages/kit/src/core/sync/write_types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ function update_types(config, routes, route, to_delete = new Set()) {
);
// null & {} == null, we need to prevent that in some situations
declarations.push(`type EnsureDefined<T> = T extends null | undefined ? {} : T;`);
// Takes a union type and returns a union type where each type also has all properties
// of all possible types (typed as undefined), making accessing them more ergonomic
declarations.push(
`type OptionalUnion<U extends Record<string, any>, A extends keyof U = U extends U ? keyof U : never> = U extends unknown ? { [P in Exclude<A, keyof U>]?: never } & U : never;`
);
}

if (route.leaf) {
Expand Down Expand Up @@ -402,7 +407,7 @@ function process_node(node, outdir, is_page, proxies, all_pages_have_load = true
proxy
);

data = `Expand<Omit<${parent_type}, keyof ${type}> & EnsureDefined<${type}>>`;
data = `Expand<Omit<${parent_type}, keyof ${type}> & OptionalUnion<EnsureDefined<${type}>>>`;

const output_data_shape =
!is_page && all_pages_have_load
Expand Down Expand Up @@ -438,7 +443,7 @@ function process_node(node, outdir, is_page, proxies, all_pages_have_load = true
? `./proxy${replace_ext_with_js(path.basename(file_path))}`
: path_to_original(outdir, file_path);
const type = `Kit.AwaitedProperties<Awaited<ReturnType<typeof import('${from}').load>>>`;
return expand ? `Expand<${type}>` : type;
return expand ? `Expand<OptionalUnion<EnsureDefined<${type}>>>` : type;
} else {
return fallback;
}
Expand Down
85 changes: 14 additions & 71 deletions packages/kit/src/core/sync/write_types/index.spec.js
Original file line number Diff line number Diff line change
@@ -1,35 +1,19 @@
// @ts-nocheck
import path from 'path';
import fs from 'fs';
import { format } from 'prettier';
import { fileURLToPath } from 'url';
import { test } from 'uvu';
import * as assert from 'uvu/assert';
import { rimraf, walk } from '../../../utils/filesystem.js';
import { rimraf } from '../../../utils/filesystem.js';
import options from '../../config/options.js';
import create_manifest_data from '../create_manifest_data/index.js';
import { tweak_types, write_all_types } from './index.js';
import { execSync } from 'child_process';

const cwd = fileURLToPath(new URL('./test', import.meta.url));

function format_dts(file) {
// format with the same settings we use in this monorepo so
// the output is the same as visible when opening the $types.d.ts
// files in the editor
return format(fs.readFileSync(file, 'utf-8'), {
parser: 'typescript',
useTabs: true,
singleQuote: true,
trailingComma: 'none',
printWidth: 100
});
}

/**
* @param {string} dir
* @param {(code: string) => string} [prepare_expected]
*/
async function run_test(dir, prepare_expected = (code) => code) {
async function run_test(dir) {
rimraf(path.join(cwd, dir, '.svelte-kit'));

const initial = options({}, 'config');
Expand All @@ -43,66 +27,25 @@ async function run_test(dir, prepare_expected = (code) => code) {
config: /** @type {import('types').ValidatedConfig} */ (initial)
});
await write_all_types(initial, manifest);

const expected_dir = path.join(cwd, dir, '_expected');
const expected_files = walk(expected_dir, true);
const actual_dir = path.join(
path.join(cwd, dir, '.svelte-kit', 'types'),
path.relative(process.cwd(), path.join(cwd, dir))
);
const actual_files = walk(actual_dir, true);

assert.equal(actual_files, expected_files);

for (const file of actual_files) {
const expected_file = path.join(expected_dir, file);
const actual_file = path.join(actual_dir, file);
if (fs.statSync(path.join(actual_dir, file)).isDirectory()) {
assert.ok(fs.statSync(actual_file).isDirectory(), 'Expected a directory');
continue;
}

const expected = format_dts(expected_file);
const actual = format_dts(actual_file);
const err_msg = `Expected equal file contents for ${file} in ${dir}`;
assert.fixture(actual, prepare_expected(expected), err_msg);
}
}

test('Create $types for +page.js', async () => {
test('Creates correct $types', async () => {
// To safe us from creating a real SvelteKit project for each of the tests,
// we first run the type generation directly for each test case, and then
// call `tsc` to check that the generated types are valid.
await run_test('simple-page-shared-only');
});

test('Create $types for page.server.js', async () => {
await run_test('simple-page-server-only');
});

test('Create $types for page(.server).js', async () => {
await run_test('simple-page-server-and-shared');
});

test('Create $types for layout and page', async () => {
await run_test('layout');
});

test('Create $types for grouped layout and page', async () => {
await run_test('layout-advanced');
});

test('Create $types with params', async () => {
await run_test('slugs', (code) =>
// For some reason, the order of the params differentiates between windows and mac/linux
process.platform === 'win32'
? code.replace(
'rest?: string; slug?: string; optional?: string',
'optional?: string; rest?: string; slug?: string'
)
: code
);
});

test('Create $types with params and required return types for layout', async () => {
await run_test('slugs');
await run_test('slugs-layout-not-all-pages-have-load');
try {
execSync('pnpm testtypes', { cwd });
} catch (e) {
console.error(/** @type {any} */ (e).stdout.toString());
throw new Error('Type tests failed');
}
});

test('Rewrites types for a TypeScript module', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,9 @@
// test to see if layout adjusts correctly if +page.js exists, but no load function

/** @type {import('../.svelte-kit/types/src/core/sync/write_types/test/layout-advanced/(main)/$types').PageData} */
const data = {
root: ''
};
data.root;
// @ts-expect-error
data.main;
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
export function load() {
/** @type {import('../../.svelte-kit/types/src/core/sync/write_types/test/layout-advanced/(main)/sub/$types').PageLoad} */
export async function load({ parent }) {
const p = await parent();
p.main;
p.root;
// @ts-expect-error
p.sub;
return {
sub: 'sub'
};
}

/** @type {import('../../.svelte-kit/types/src/core/sync/write_types/test/layout-advanced/(main)/sub/$types').PageData} */
const data = {
main: '',
root: '',
sub: ''
};
data.main;
data.root;
data.sub;

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export function load() {
/** @type {import('./.svelte-kit/types/src/core/sync/write_types/test/layout/$types').LayoutLoad} */
export function load({ data }) {
data.server;
// @ts-expect-error
data.shared;
return {
shared: 'shared'
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @type {import('./.svelte-kit/types/src/core/sync/write_types/test/layout/$types').LayoutServerLoad} */
export function load() {
return {
server: 'server'
Expand Down
14 changes: 13 additions & 1 deletion packages/kit/src/core/sync/write_types/test/layout/+page.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
export function load() {
/** @type {import('./.svelte-kit/types/src/core/sync/write_types/test/layout/$types').PageLoad} */
export function load({ data }) {
data.pageServer;
// @ts-expect-error
data.pageShared;
return {
pageShared: 'pageShared'
};
}

/** @type {import('./.svelte-kit/types/src/core/sync/write_types/test/layout/$types').PageData} */
const data = {
shared: 'asd',
pageShared: 'asd'
};
data.shared;
data.pageShared;
Loading