Skip to content

Commit

Permalink
[fix] better type generation for load functions with different return…
Browse files Browse the repository at this point in the history
… values (#7425)

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

Fixes #7408

Also reworking write_types tests to type-check the outcome, not compare the generated code

* explanation comment

* fix excludes
  • Loading branch information
dummdidumm committed Oct 28, 2022
1 parent 934db68 commit edd815e
Show file tree
Hide file tree
Showing 39 changed files with 223 additions and 688 deletions.
5 changes: 5 additions & 0 deletions .changeset/friendly-pears-wash.md
@@ -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
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
@@ -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
@@ -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;
@@ -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.

@@ -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
@@ -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
@@ -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;

0 comments on commit edd815e

Please sign in to comment.