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

Generated types #4120

Merged
merged 22 commits into from
Mar 2, 2022
Merged

Generated types #4120

merged 22 commits into from
Mar 2, 2022

Conversation

Rich-Harris
Copy link
Member

This code is a bit ratty, but I figured I'd push it up anyway. It generates .d.ts files for every page/endpoint file in src/routes (but tucked away in .svelte-kit via the magic of rootDirs). Easier to show than tell:

image

An eventual follow-up would be to automatically type Props (and the return value of a shadow endpoint, if no load function is given) based on a component's <script> tag.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2022

🦋 Changeset detected

Latest commit: 91e1a66

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

@Rich-Harris
Copy link
Member Author

Rich-Harris commented Feb 28, 2022

This is ready for review but lint is failing because pnpm check apparently runs before the test apps have been built (which creates the .svelte-kit/tsconfig.json files that pnpm check depends on). Not sure why, since the job includes pnpm build:

Lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: pnpm/action-setup@v2.2.1
with:
version: 6.23.2
- uses: actions/setup-node@v3
with:
node-version: '14.x'
cache: pnpm
- run: pnpm install --frozen-lockfile
- run: pnpm build --filter ./packages --filter !./packages/create-svelte/templates
- run: pnpm lint
- run: pnpm check
Any ideas?


update: think i just needed to update pnpm-workspace.yaml

@pheuter
Copy link

pheuter commented Mar 2, 2022

Would it be fair to understand these changes as striving towards the direction of something like tRPC where you have the client being able to use types auto-generated from server endpoints?

baseplate-admin added a commit to coreproject-moe/CoreProject that referenced this pull request Mar 3, 2022
@CoolOppo
Copy link

CoolOppo commented Mar 4, 2022

My tsconfig currently has this in it:
"extends": "@tsconfig/svelte/tsconfig.json",

Looking at the changes though, it appears I'm doing something wrong. How do I use the svelte-kit version?

@thenbe
Copy link
Contributor

thenbe commented Mar 4, 2022

Would it be fair to understand these changes as striving towards the direction of something like tRPC where you have the client being able to use types auto-generated from server endpoints?

@pheuter It seems like it, to me at least, given that #647 is included in the 1.0 milestone.

Although, that may just be wishful thinking on my part. I recently added tRPC to my sveltekit app, in what I can only describe as the cherry to the delicious cake that is sveltekit. The effortless type safety has made what was already a ridiculously fun dev experience even better. Needless to say, I'd be pretty excited if sveltekit were to move in that direction.

And I mention tRPC in particular because of how simple or lightweight their approach is, which I feel aligns with svelte's own. In contrast, my original approach of implementing "end-to-end" typesafety was through graphQL. And while it could lead to the same result, the bloat accompanying graphQL quickly gets out of hand where you start questioning whether you're actually improving the dx at all. It ends up sticking out like a sore thumb alongside the simplicity of your typical sveltekit app.

P.S. I'm not an expert on the matter by any means, do take what I say with a grain of salt.

@BrettBedarf
Copy link

BrettBedarf commented Mar 6, 2022

Is this intended to work the same for .ts files in addition to .js with jsdoc comments?

I am able to import the generated RequestHandler type from its full project path but not relative.

image

Minimal example: https://github.com/BrettBedarf/generated-types-minimal

/routes/ts/[slug].json.ts vs /routes/js/[slug].json.js

I think the issue is typescript trying to resolve the same file name does not "merge" the import

If I manually change the generated .d.ts file to [slug].json.gen.d.ts and change the import to import type { RequestHandler } from './[slug].json.gen'; I am able to use the type

@thenbe
Copy link
Contributor

thenbe commented Mar 8, 2022

@BrettBedarf I'm seeing a similar issue. Eslint is reporting that the file is importing itself. It seems that an endpoints with a .ts extension wil try to import itself rather than importing it's generated types.

In the meantime, try the following workaround. Basically just add a .d to the import.

- import type { RequestHandler } from './[slug].json';
+ import type { RequestHandler } from './[slug].json.d';

@BrettBedarf
Copy link

@ambiguous48 good workaround thanks!

@TravisSpomer
Copy link
Contributor

Is this intended to work the same for .ts files in addition to .js with jsdoc comments?

It doesn't seem so—seems like a really strange oversight. But you can hack it to work in TypeScript like this (at least for pages):

import type { Load } from "./filename"
export async function load({ params }: Parameters<Load>[0]): Promise<ReturnType<Load>>
{ /* ... */ }

If the generated types file exported LoadInput and LoadOutput then it would be a lot simpler.

@accuser
Copy link

accuser commented Apr 12, 2022

Importing the RequestHandler in TypeScript works if you are explicit about the path:

import type { RequestHandler } from '.svelte-kit/types/src/routes/[slug]';

export const get: RequestHandler = async ({ params }) => {
  const { slug } = params;

I'm assuming that the rootDirs configuration in .svelte-kit/tsconfig.ts isn't working as expected.

baseplate-admin added a commit to coreproject-moe/CoreProject that referenced this pull request May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.