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

Svelte 5: default property value should be real and not initial|undefined #9948

Closed
dm-de opened this issue Dec 17, 2023 · 4 comments · Fixed by #10797
Closed

Svelte 5: default property value should be real and not initial|undefined #9948

dm-de opened this issue Dec 17, 2023 · 4 comments · Fixed by #10797
Milestone

Comments

@dm-de
Copy link

dm-de commented Dec 17, 2023

Describe the problem

3 months ago, I posted this:
https://www.reddit.com/r/sveltejs/comments/16qeg71/svelte_5_fixed_props_issue_you_may_not_aware_of_it/
I was happy about this

It looks like Svelte 5 fixed default props issue. I think most people are not aware about that "export let" with default value is initial value (like constructor), but not default value as replacement for undefined.

Now, I found out that this was changed again.
Most people expect that if undefined is passed, the default value will be used. And they are then surprised when this is not the case.

For comparison:
React have real default values:
const {fontSize = "20px"} = props;
https://blog.logrocket.com/complete-guide-react-default-props/

Vue have real default values (see code below).
const props = defineProps({ name: {type: String, default: 'Anonymous'} })
https://hackernoon.com/how-to-set-default-value-of-props-in-vue

solid-js have real default values
const merged = mergeProps({ greeting: "Hi", name: "John" }, props);
https://www.solidjs.com/tutorial/props_defaults?solved

Describe the proposed solution

Simple make default and not initial again.

Demo Svelte 4:
https://svelte.dev/repl/393c4caef5fe4297a00da4b7ab9fd4d9?version=4.2.8

Demo Svelte 4 - but with default values as WORKAROUND (using fallback at place)
https://svelte.dev/repl/55e8468acb474369931de40d489c0121?version=4.2.8

Demo Svelte 5 - but with default values as WORKAROUND (using $:)
https://svelte.dev/repl/649275e475184354a6316111cb451ee3?version=4.2.8

Demo Svelte 5:
LINK

Demo Svelte 5 - but with default values as WORKAROUND (using fallback at place)
LINK

Demo Svelte 5 - but with default values as WORKAROUND (using $effect)
LINK

Alternatives considered

Vue have real default values and not initial values. Test this yourself:

LINK

Importance

svelte would be better with it

@Malix-Labs
Copy link

Now, I found out that this was changed again.

Have you found which commit have that effect?

@Malix-Labs
Copy link

Malix-Labs commented Jan 2, 2024

Strong agree.

I think it's more important than "svelte would be better with it", as it could be the cause of #9764 (which, to me, is a mistake)

@dm-de
Copy link
Author

dm-de commented Jan 2, 2024

Now, I found out that this was changed again.

Have you found which commit have that effect?

Date of Reddit post is 23. Sep. 2023
Date of 1st Svelte 5 release on GITHUB is: 10. Nov. 2023
I think this wa a pre-proxy release.
I remember, at this time, I used online REPL.

@Malix-Labs
Copy link

See #9764 (comment)

@dummdidumm dummdidumm added this to the 5.0 milestone Mar 5, 2024
Rich-Harris added a commit that referenced this issue Mar 22, 2024
* breaking: apply fallback value every time in runes mode

closes #9948

* apply fallback value in setter

* encapsulate fallback logic

* we should keep this logic out of b.set, since it's very specific to accessors

* oops

---------

Co-authored-by: Rich Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants