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

Default props #5673

Closed
omerman opened this issue Nov 11, 2020 · 8 comments
Closed

Default props #5673

omerman opened this issue Nov 11, 2020 · 8 comments

Comments

@omerman
Copy link

omerman commented Nov 11, 2020

Describe the bug
When declaring a prop, export myProp, and adding a default value to it, export myProp = 5.
If a component passes a value and later on changes the value to be undefined,
myProp becomes undefined instead of becoming the default value, 5 in this case.

Attached a simple repl to display the issue in detail.
https://svelte.dev/repl/861015c678f04ade8b8a5eb8c96c36b9?version=3.29.7

Expected behavior
I believe this should be considered as a bug, since Im declaring that the prop has a default value.
Otherwise the default value is just considered as default value for the first time you mount.. and that is almost useless.

I ended up doing stuff like this to achieve my goal:

export let k = 5;
$: k = k ?? 5;

This kind of beats the point of default values.

What do you think?

Looking forward to hearing from you,
Thank you for this amazing language

@Conduitry
Copy link
Member

This has come up before in #4442 and there were practical implementation reasons to leave it how it was - and the decision was to document it rather than change it. https://svelte.dev/docs#1_export_creates_a_component_prop now explicitly mentions that this is an initial value rather than a default one, and that changing it to undefined does not revert it to its initial value.

As I'm writing this though, I see now that in https://svelte.dev/tutorial/default-values we're still using the word 'default', so that should be changed as well, and I'm labeling this as a documentation issue.

@dummdidumm
Copy link
Member

From my point of view it would be highly confusing if this would be a fallback, not initial value. So I also prefer how it's currently implemented.

@omerman
Copy link
Author

omerman commented Nov 11, 2020

-- Edited this comment, check the bottom paragraph --

@Conduitry I understand what you are saying, and still, there is a fundamental issue with this behavior.

I will show you my point with typescript. In the example myProp is considered as a number, not number | undefined.
because it has the initial value we claim it to be considered as.

Svelte compiler tells me that I can render Comp without supplying a value to myProp, so Im getting the feeling that either it has a default value, or it is ok to just not be defined.

As you can see in the example, after 5 seconds, it will display NaN on the screen and it wouldn't be the fault of the App developer..
In my opinion, it's a bug..

Don't you agree?

<script lang="ts">
//Comp.svelte
export let myProp: number = 5;
</script>

<div>{myProp / 3}</div>
<script>
// App.svelte
let bool = true;

setTimeout(() => {bool = false}, 5000);
</script>

{#if bool}
<Comp myProp={1} />
{:else}
<Comp />
{/if}

@Conduitry I checked it again. I was wrong, the case I presented above would actually work as I expect. So In this case, I guess I fully understand and this is truly not a bug. I do think there should be a way then to have a default value behavior. Its very basic..
Isn't it?

@dummdidumm
Copy link
Member

dummdidumm commented Nov 11, 2020

You can achieve a default value like this

<script>
   export let myProp;
   $: myProp = myProp ?? 5;
</script>

@omerman
Copy link
Author

omerman commented Nov 11, 2020

@dummdidumm Yes thats what Im doing today. I just wanted something more.
But what I want now that I understand that its not a bug.. is a compiler solution for this. I'll think about it some more, Maybe I'll open a different feature request.

Thank you @dummdidumm @Conduitry

@omerman
Copy link
Author

omerman commented Nov 11, 2020

@Conduitry @dummdidumm Closing, but I still want to state that I think Svelte should have a way to declare a default value, and not just an initial value. It doesn't have to be a breaking change, it can be introduced as a new syntax, Im sure a lot of people would use it. I know I would.

@omerman omerman closed this as completed Nov 11, 2020
@macmillen
Copy link
Contributor

@Conduitry @dummdidumm
I also think this is an unintuitive behavior. When you are coming from any other frontend framework you expect this to be a default value and not an initial value. It just changes the whole type. If I create a prop like this:

  export let status: ButtonStatus = "primary";

the actual type is:

var status: ButtonStatus | undefined

and not

var status: ButtonStatus

which is weird to me because it differs from the TypeScript / JavaScript language implementation.

Also this solution I don't think is a good idea since it introduces unnecessary bloat which Svelte normally tries to prevent:

<script>
   export let myProp;
   $: myProp = myProp ?? 5;
</script>

There is a similar behavior being fixed in the next TypeScript version (4.4) for interfaces. Does that maybe have something to do with it? microsoft/TypeScript#13195

@dm-de
Copy link

dm-de commented Mar 25, 2023

This is an important and dangerous thing!
You need to write a blinking alert block in docs, so that is properly understood.

I use Svelte since 2020, but default prop are not, what i expected all the time!
All the time I understood it not like init, but like function default value!
So I need to review my code again!

It is a bit dangerous, because i thought, that Svelte will use always default value for prop, if value is not set - so I never check props for undefined (nearly all my props have a default value) !!!

Problematic specially with arrays/objects

App.svelte:
<script>
	import Component from './Component.svelte'
	let items = [1,2,3]
</script>

<button on:click={() => items=undefined}>Button</button><br>
<Component {items} />
Component.svelte:
<script>
	export let items = []
</script>

{#each items as item}
{item}<br>
{/each}

best workaround is to do this:

export let items; $: items || []

As you can see - this looks ugly

Svelte could do this automagically, you could add option flag or something

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

No branches or pull requests

5 participants