-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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: Tell if a bindable prop is actually passed using bind #11672
Comments
What's the advantage of doing this instead of just doing <!-- MyComponent.svelte -->
<script>
let { values = $bindable(), onChange } = $props();
const addValue = (newValue) => {
values ||= [];
values.push(newValue);
onChange?.(values);
};
</script>
....
<!-- Somewhere -->
<MyComponent bind:values={...}>
<!-- Somewhere else -->
<MyComponent values={...} onChange{...}> this way you also avoid creating new memory by spreading. |
You can test it out, it triggers a warning and it has side effect. It shouldn't update values because I didn't use bind. To get rid of this error, I would be forced to create a "memory spreading" as you call it to avoid that side effect
|
How about something like: let { prop = $bindable.required() } Maybe a different name like
Child components will want to implement one or both of these behaviours:
If only one of the behaviours is needed, either a callback function (events) or bindable prop with no default is used. If you need both, use a bindable prop with a default value. The problem is, as a child component, if you have a bindable prop, you will mutate it, otherwise it wouldn't be bindable. I believe the warning doesn't really make sense and might not catch potential issues fast enough (parent doesn't bind the prop but the child rarely mutates it so the warning never pops up). I think there are a few solutions to this:
Whatever you thoughts on everything I said here are, my only question is: what is the use case for allowing |
Hi, These are indeed very good suggestions! However, I'm on the fence of disagreement that a bindable prop should always be binded. I believe the very best case scenario should be
The reason is that I believe it should be possible to have a component that can either manage its own state or manage an external state with bind. Let's take an exemple of a "Svelte Select Library" named "MySelect" // Component mutates and always display the current selection based on value prop
<MySelect bind:value={value} options={...}>
// Component uses its own state to manage the value. When the form is submitted, it will be submitted like a normal form input.
<MySelect name="user[firstName]" options={...}>
// Component uses its own state to manage the value. A callback onChange is used to do something with the selected value
<MySelect value={initialValue} options={...} onChange={/* do something */}> |
@trueadm If this just got added to the milestone 5.0, does this mean it's considered as an "accepted" request? I was wondering what could be done to drag more attention into this as I believe this is kind of important in some case scenario |
It means we're going to look into the issue, not that the request is accepted per say :) |
Understood thanks! By the way I would love to have your opinion on #11672 (comment) above |
We really want to avoid adding more runes here where possible. If possible, bindings should preferably be transparent and not something you need to worry about. So there's likely an alternative approach to dealing with this. |
Describe the problem
Svelte 5 warns when non-binded props are mutated which is a very good thing to avoid unexpected side effect.
However, there is no way to tell if a prop is actually binded or not, or at least if there is it doesn't appear to be documented.
Shallow/deep copy could be avoided in some scenarios where performance matter if it's not even used.
Describe the proposed solution
Here is a real life possible scenario where you might just want to call a callback with the new values without mutating the actual passed value.
Importance
must have.
Svelte 4 librairies should be able to tell that as well to preserve compatibility without switching to runes
The text was updated successfully, but these errors were encountered: