-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: make render
options optional
#12111
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
Conversation
🦋 Changeset detectedLatest commit: 8ce0a54 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 |
We need to take into account the "there are required properties" case, at which point they are required. Probably needs a function overload to make that happen. Possibly same for |
Can we tell TypeScript that omitting the argument is equivalent to passing |
I think it's doable with something like this (it's also already used for the actions if i'm not mistaken. Dunno if it's doable checking if all the properties are optional tho. EDIT:
EDIT AGAIN: |
I took a shot at implementing it but feel free to either steal and close or change whatever you want |
I think something along the lines of this should do the trick: declare function render<
T extends SvelteComponent<any> | Component<any>,
Props extends ComponentProps<T> = ComponentProps<T>,
>(
component: T extends SvelteComponent<any> ? ComponentType<T> : T,
options: { props: Props }
): void;
declare function render<
T extends SvelteComponent<any> | Component<any>,
Props extends ComponentProps<T> = ComponentProps<T>,
>(
component: {} extends Props ? T extends SvelteComponent<any> ? ComponentType<T> : T : "second argument to render function is required",
options?: { props: Props }
): void; |
|
Oh wow...I spent a bunch of time for nothing yesterday lol. I guess is because {} comes first |
Yeah |
* Only available on the server and when compiling with the `server` option. | ||
* Takes a component and returns an object with `body` and `head` properties on it, which you can use to populate the HTML when server-rendering your app. | ||
*/ | ||
export function render< |
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.
had to move this into a d.ts
file - JSDoc was too limiting to properly type this without type errors
make the props parameter optional only when there are no or only optional props --------- Co-authored-by: Simon Holthausen <[email protected]> Co-authored-by: Simon H <[email protected]>
no need to make people do
render(Thing, { props: {} })
instead ofrender(Thing)
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint