-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
semi public types #4104
semi public types #4104
Conversation
🦋 Changeset detectedLatest commit: a9cb86e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Ok I think this is ready for review. There's some further tweaks I'd like to make to the types (particularly The big change here, other than that private types are now documented, is that a bunch of formerly public types are now in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great!
packages/kit/types/internal.d.ts
Outdated
type ToJSON = { toJSON(...args: any[]): Exclude<JSONValue, ToJSON> }; | ||
|
||
export type TrailingSlash = 'never' | 'always' | 'ignore'; | ||
// TODO should this be public? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe remove this TODO now that it's no longer public?
- Update Svelte Kit types to account for changes in sveltejs/kit#4104
This comment was marked as spam.
This comment was marked as spam.
By unexporting LoadInput/LoadOutput, this change made using Typescript harder. Previously, I could say
Now it seems I have to use this form
I know this is a style thing, and a lot of people seem to write arrow expressions everywhere, but I'm more inclined to agree with https://deno.land/manual/contributing/style_guide#top-level-functions-should-not-use-arrow-syntax |
I'm not disagreeing about whether we should export
|
Sure, that avoids the arrow expression, but still uses an unnecessary const where just |
- Update Svelte Kit types to account for changes in sveltejs/kit#4104
Deploy preview: https://kit-svelte-dev-git-semi-public-types-svelte.vercel.app/docs/types#additional-types
I tried to do this in #4021 but the merge was too gnarly. Easier to start fresh.
The basic thinking here is that we have public types in
index.d.ts
andambient.d.ts
, and private-but-documented types inprivate.d.ts
. Changes to these types should be made carefully to avoid breakage.internal.d.ts
is for things that aren't referenced by public types, and can be changed freely.So far I haven't actually moved most of the types, I've just set up the logic for extracting/rendering/linking them.