-
-
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
feat: add PageProps
to $./types
#12967
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 4ad2dc8 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 |
preview: https://svelte-dev-git-preview-kit-12967-svelte.vercel.app/ this is an automated message |
Did you accidentally forget to commit the changes? This PR only modifies the documentation |
I'm pretty sure I changed the code. I will check now. Thansk for the notice! |
@weebao I think you meant to make the changes in the file below before regenerating the types. kit/packages/kit/src/types/ambient.d.ts Lines 36 to 46 in d94a854
The types/index.d.ts file will get replaced based on the changes to this file.
|
Why have the "data" in the props? Wouldn't it be even cleaner to emit it? |
That's because I want to not keep writing the code below on every page, so this is what I proposed. <script>
import { PageData } from './$types';
interface PageProps {
data: PageData;
}
let { data }: PageProps = $props();
</script> Is this the correct way of doing it? This is my first time submitting a PR to this codebase so please let me know if I'm doing terribly wrong 😅 |
What about something like this? // +page.ts
export const load = () => {
return {
framework: "svelte",
version: 5
}
} <script lang="ts">
// +page.svelte
import { PageProps } from './$types';
let { framework, version }: PageProps = $props()
</script> A |
Ohhh, I get your point. Having the I think for your example case, the user would have to define their own CustomPageProps instead of importing from the global |
Before spending time implementing it we should get an approval on the API from some of the svelte(kit) maintainers @Rich-Harris @dummdidumm @benmccann |
packages/kit/src/types/ambient.d.ts
Outdated
@@ -7,6 +7,7 @@ | |||
* // interface Error {} | |||
* // interface Locals {} | |||
* // interface PageData {} | |||
* // interface PageProps {} |
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 doesn't belong here — PageProps
isn't part of the App
namespace, it's part of ./$types
* // interface PageProps {} |
packages/kit/src/types/ambient.d.ts
Outdated
/** | ||
* Defines the reusable shape for [$props()](https://svelte.dev/docs/svelte/$props#Type-safety) for `+page.svelte` specifically | ||
*/ | ||
export interface PageProps { | ||
data: PageData; | ||
} | ||
|
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.
ditto
/** | |
* Defines the reusable shape for [$props()](https://svelte.dev/docs/svelte/$props#Type-safety) for `+page.svelte` specifically | |
*/ | |
export interface PageProps { | |
data: PageData; | |
} |
Hi — thanks for the PR but I'm afraid this isn't the right way to do it — export type PageProps = { data: PageData, form: ActionData, children: Snippet }; (Note that it includes |
@Rich-Harris That actually makes so much sense. I'll work on it now |
@@ -465,6 +465,12 @@ function process_node(node, outdir, is_page, proxies, all_pages_have_load = true | |||
|
|||
exports.push(`export type ${prefix}Data = ${data};`); | |||
|
|||
exports.push( | |||
`export type ${prefix}Props = { data: ${prefix}Data; ${ | |||
node.server && is_page ? 'form: ActionData; ' : '' |
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.
Pretty dumb way to write this I know. I could've pushed this in the nested if above but I just thought writing it below PageData
would be more intuitive. Also, didn't know how to add children
in LayoutProps
since I can't write import("svelte").Snippet
as svelte isn't included.
Closes #12726
Added a
PageProps
type for defining page props using the$props()
rune.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits