svelte-kit package: Generate type definitions

[dummdidumm]

WIP
Closes #1628
Fixes #1717

TODO

• generate only the files from within the lib folder, not others. Not sure how to best accomplish this. Either postprocess and remove all definition files from other places, or manipulate the tsconfig prior to the emit in a way that it only traverses the files inside the lib folder
• add types to the generated package.json
• Move some postprocessing of the generated tsx into svelte2tsx

Open questions:

• Does svelte2tsx become a dependency, a peer dependency, or nothing of that? It's needed to generate the type definitions

[dummdidumm]

Apart from a little cleanup and the types output option this is now ready to review.

One question that is left for me is: Do we want to add types by default? - which would mean the user has to install TypeScript and svelte2tsx even in JS-only projects. I'd probably say "yes" to force package authors to be a good citizen of the ecosystem and to provide types.

[ignatiusmb]

Do we want to add types by default?

Absolutely, and yes to force authors to be a good citizen of the ecosystem. Svelte components exported from a js file without types is (at least) usable but not the best experience for regular users, and virtually unusable for TS users. Importing the svelte files itself is a workaround, but with exports map, the imports are essentially locked so no importing source files directly.

Should they install TypeScript and svelte2tsx themselves? Can't we automatically install or add it for them when they run svelte-kit package command?

[dummdidumm]

Should they install TypeScript and svelte2tsx themselves? Can't we automatically install or add it for them when they run svelte-kit package command?

I'm not sure we can because people could be using npm, yarn, pnpm, or whatever to do this, and I'm not sure there's a good way to detect this.

This is now ready for review (with a pending answer to the svelte2tsx/typescript question), I'll add a changeset soon.

[ignatiusmb]

That's true. Also, some docs update would be nice.

[changeset-bot]

🦋 Changeset detected

Latest commit: 22c57ca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dummdidumm]

I split up the types option into one for the folder and one for the entry point, because they not necessarely have to correlate with each other. I also added some docs and a changeset, so this should be good to merge if there are no objections to the way types are forced by default and the requirement to instal typescript/svelte2tsx by hand.

[ignatiusmb]

Looking good, can't wait for this to be merged!

[bluwy]

Curious if generate-dts.js could be extracted as a package under language-tools? Would be nice if the dts creation logic can be officially supported without locking into SvelteKit. It would lower the barrier for the ecosystem to provide types.

[benmccann]

@dummdidumm it looks like this one will need a rebase

[dummdidumm]

Jsdoc->dts should work now, too. I'll try adding a test later for all of this.

Regarding the overwrite: I was about to write "To me this feels like something that is the user's fault and they need to structure the exports differently", but then I thought "how do I even do that without TypeScript files, if I only use JS and dts files?", so that's a good question. The only thing that comes to my mind is to append the manual d.ts file content to the generated one. Or the user is expected to write a little bit of duplicated code by doing
index.js:

export { default as Circle } from './Circle.svelte';
export { default as Square } from './Square.svelte';

index.d.ts

export { default as Circle } from './Circle.svelte';
export { default as Square } from './Square.svelte';
export type Point = {
  x: number;
  y: number;
};

[dummdidumm]

Added tests, which uncovered some more subtle bugs, and added a warning if a generated d.ts file is overwritten by a handwritten file. @Rich-Harris I think in this case its best to place the burden on the author to enhance his d.ts file and have a little duplication. The alternative of automatically appending the handwritten content to the generated one might not always be what you want.

This is now ready to merge, although it would be great if you could test this out on your own package before merging it @Rich-Harris .

[Rich-Harris]

I tried this out on an unreleased library and it worked great once I figured out that I needed to replace my jsconfig.json with a tsconfig.json (not sure if there's a way to make it accept either?).

But when I used the library with the freshly generated types, I started seeing errors like this:

The <Group> component referred to there looks like this (i.e. it's a JSDoc component):

<script>
	import * as THREE from 'three';
	import { setup } from '../../utils/context.js';
	import { transform } from '../../utils/object.js';
	import * as defaults from '../../utils/defaults.js';

	export let position = defaults.position;
	export let rotation = defaults.rotation;
	export let scale = defaults.scale;
	export let renderOrder = 0;

	const { root, self } = setup(new THREE.Group());

	$: {
		self.renderOrder = renderOrder;
		transform(self, position, rotation, scale);
		root.invalidate();
	}
</script>

<slot></slot>

The generated declaration looks like this:

/** @typedef {typeof __propDef.props}  GroupProps */
/** @typedef {typeof __propDef.events}  GroupEvents */
/** @typedef {typeof __propDef.slots}  GroupSlots */
export default class Group {
}
export type GroupProps = typeof __propDef.props;
export type GroupEvents = typeof __propDef.events;
export type GroupSlots = typeof __propDef.slots;
declare const __propDef: {
    props: {
        position?: any;
        rotation?: any;
        scale?: any;
        renderOrder?: number;
    };
    events: {
        [evt: string]: CustomEvent<any>;
    };
    slots: {
        default: {};
    };
};
export {};

I'm not sure what the relationship between __propDef and $$prop_def is, but it feels like there might be a version mismatch somewhere? Any idea how to diagnose this?

[dummdidumm]

Found the reason why this wasn't working for jsconfig files and in your case for the tsconfig. Turns out we need to force some more TS config options. Tested this on your project and it works now as expected.
I also moved the functionality of emitting dts files into svelte2tsx so others outside of SvelteKit can use it if they want to.

Merging as this feels battle-tested enough now.

[janosh]

Even if emitting types is the default, there should definitely be an option to disable them. How about config.kit.package.emitTypes? Happy to PR if you guys agree.

[dummdidumm]

Could you elaborate why this should be configurable? I see no harm in always generating types as long as they are sound.

[janosh]

Because of the added dependencies on svelte2tsx and typescript.

[dummdidumm]

You only need those when authoring packages, for which I think it is best for the ecosystem quality to generate types, always. Or do you see use cases where this would be bad?
One case I can get behind is if you decide to provide hand written types, in which case auto generation gets in your way. Would that be the case for you?

[janosh]

I'm all for types! There's no use case where I think they would be bad except maybe people building packages only for personal use who don't want to deal with types. But I think the incentive to add types is strong enough if you enable them by default. There should still be an escape hatch for unforeseen scenarios. When documenting such a new option, you could simply include your statement

We believe it is best for ecosystem quality to generate types, always. Please make sure you have a good reason to author a package without types when setting config.kit.package.emitTypes: false.

or something along those lines.

[dummdidumm]

Sounds good. If you want to create the PR, this commit can serve as inspiration where to make the changes for the option. Also a test would be nice.