Skip to content

Svelte 5 - being able to tell which props are bound to #10659

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

Closed
rChaoz opened this issue Feb 27, 2024 · 13 comments
Closed

Svelte 5 - being able to tell which props are bound to #10659

rChaoz opened this issue Feb 27, 2024 · 13 comments
Labels
awaiting submitter needs a reproduction, or clarification

Comments

@rChaoz
Copy link
Contributor

rChaoz commented Feb 27, 2024

Describe the problem

Currently, you can always mutate exported let variables (props), regardless of them being bound to or not.
In Svelte 5, you can only modify prop variables if they are bound to. This is a great change, but sometimes you might want to both (1) have props that binding to is optional and (2) modify said props. This is now not allowed. A check before attempting to edit the prop would be very useful.

Describe the proposed solution

There could be something like a $bound(prop) statement, which returns boolean. This wouldn't mean any extra runtime/static checks, just a tool for the component builder.

Sample use case: custom input field. When value={x} is passed, the field is in a read-only state. When bind:value={x}, the field would be editable. A mechanism to indicate which props you can (or even must) bind to would also be nice, but for now I think this alone would be a great addition.

Importance

would make my life easier

@Prinzhorn
Copy link
Contributor

I'm confused, the docs say:

Props cannot be mutated, unless the parent component uses bind:. During development, attempts to mutate props will result in an error.

But I can't trigger the error in REPL (assuming it runs in dev mode).

I had no idea that this limitation exists with runes, I have many components where bindings are optional. E.g. a ResizableTextarea where you can bind to height if you want to. E.g. for some of them I store the height so that I can restore the UI when you come back the next day and I bind:height={$settings.height}. For others I still want them to be resizable but I don't need to restore the size because they're in some dynamic part of the app. These will just default to the defaultHeight prop and height is simply written into the the abyss, because the parent doesn't declare that prop at all.

This also refs #8218 because apparently there are now two types of props and not three?

This wouldn't mean any extra runtime/static checks, just a tool for the component builder.

The check would need to happen at runtime and also in production, it's impossible to statically determine if something will be bound. Currently this only happens in dev.

@dummdidumm
Copy link
Member

The docs are outdated, you get a warning (not an error) if you mutate a property (note: reassign is fine) that isn't bound.

As for the "check if it's bound" helper: Could you give a concrete example where you need this, where it wouldn't also be possible to reassign the property instead (e.g. foo = { ...foo, prop: 'new value' } instead of foo.prop = 'new value')?

@dummdidumm dummdidumm added the awaiting submitter needs a reproduction, or clarification label Feb 28, 2024
@Prinzhorn
Copy link
Contributor

Prinzhorn commented Feb 28, 2024

if you mutate a property (note: reassign is fine) that isn't bound.

Thanks, I get it now and all is good! Here's what I learned:

  1. This only works for $state, if I pass a regular object or $store down I can freely mutate it. This isn't clear from the docs, since $props() only covers the receiving end but it matters what the parent sends down
  2. I didn't differentiate between mutating and reassigning. It all makes sense now.

From the wording of their text I'm pretty sure @rChaoz had the exact same misunderstanding of the docs. Throwing in the word "modify" for extra confusion between assignment and mutation 😄

@rChaoz
Copy link
Contributor Author

rChaoz commented Feb 28, 2024

Regardless of the docs misunderstanding or not, I still think it would be nice to be able to tell, from a component perspective, when your props are bound or not.

@dummdidumm
Copy link
Member

In what use case would this be actually relevant / benefitial?

@rChaoz
Copy link
Contributor Author

rChaoz commented Feb 29, 2024

I think my initial use case where you can implement an input field that is read only/editable based on whether or not the "value" prop is bound to is a good example.

@figloalds
Copy link

figloalds commented Mar 4, 2024

I've been migrating and I know exactly what OP means
As of right now there is no way to tell if a prop of a component will be mutated or bound inside it, from the outside you never know which props you need to pass with bind:, there's also no way for the child component to explicitly declare that they need to mutate an input prop. This is very important for custom input components, where value changes coming from the model should affect the view, and changes coming from the view (user) must change the model back into the parent component's state

Here's a minimal repro:
https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACo1RQU7DMBD8yspCohVRAxxDUglxgQsXuBEOwXWIRbxr2euiKsqv-oK-DNkpbUEcOFiyRzM749lBtLpXXhQvg8DGKFGIW2tFJnhj48OvVc9KZMJTcDIipZdOW17WWLM2lhzDHRlLqJCvoXVkoBaL_Igtphm1iIp4esXQEkEFZ54bVrMBJAXkAi5hnN_UGEltQMmaEDRKp4xCns1hSCO4JVokBVxUcBUFPNZY5sdkWJ5kWjd9UNXQEo3L8iTYRHwLzIRAKHstP6rh4DemLybUFzAcTJPVpFqKTBha6VarlSjYBTVmhx6PRv-tMxaz_2LKDNM9z-GRuNP4Dp1yCiQZE1DLhpUHJni-f3iK4OQG5KDBDVALms89BK-cP07irmHgTvsTxafuezAh7gJ222S926ZW446sI-tn33v51fOQ2PtioMZSow0MbxpXxb73H4z8j8pexy-bdZAlhgIAAA==

As of right now, when you edit the input, you get a Runtime Exception. It would be great if this exact error became a compile-time error, with a nice little message saying: "You need to bind to this property, because this component will try to mutate it"

There's a problem that I ran into where child would receive props and they wouldn't be reactive at all, they were frozen. The input checkbox would change "checked" (which was bound to a prop), but the prop value would not update at all, I didn't manage to create a simple repro for this problem yet, but I've seen someone on Discord mention running into this problem too, it has to do with passing state props via destructuring

@brunnerh
Copy link
Member

brunnerh commented Mar 4, 2024

when you edit the input, you get a Runtime Exception

It's a warning, not an exception, and it only happens during development.

You can avoid this in general by not passing objects but primitives.
Primitives also always require the use of bind: for changes to propagate.

@dummdidumm
Copy link
Member

@figloalds I think this is not what @rChaoz meant. From what I understand, @rChaoz wants a way to know at runtime "has this property been wired up with bind: in the parent". What you want is a way to declare within the component that a property should be bound to.

The problem with that is that it's not always certain that reassigning a property means that you want the property to be bound - there are also patterns where you may want to go out of sync temporarily with the parent. For example carbon-components-svelte has an Input component where you can either do <Input bind:value /> or do <Input {value} {onChange} /> - i.e. you as the consumer can decide whether or not to use the binding, or use a property value+callback pair. In both cases, the Input component will write to value but only if it's bound it will propagate up. What we could do is have a rune like $bind() to tell which properties can be bound (example: let { regularProp, canBeBound = $bind() } = $props();, so that you'll get an error when you try to bind anything else - but I'm not sure it's worth the overhead of learning when to add that rune, to me this feels more like a documentation task for the component author (add jsdoc to explain whether or not you can/should bind to a certain property).

@figloalds
Copy link

figloalds commented Mar 7, 2024

@dummdidumm I see
In any case, trying to modify a prop that wasn't bound will fill the console with warnings as if to say "Don't do this, this is kinda bad"
I understand this use-case where you have prop that can be bound to, but can also be used as one-way binding on the parent component, I myself dislike this as it looks like React, but I understand that it's a valid use-case.

In that case, since the internal component is doing this interesting multi-modal functionality, then there should be a way that this component announces that they're doing this, so that we can get a proper warning or error when the parent component uses it wrong. But I do think we should have a bindable thing with the option to configure it to be "Requires two-way binding" or "Can bind, but doesn't require two-way binding", would be nice for my component to announce how it's used, and for the editor to tell me when I'm using a component wrong

@dummdidumm
Copy link
Member

This may be of interest for you: #10768

@Suya1671
Copy link

Suya1671 commented Apr 2, 2024

now that #10851 has been merged could this be closed?

@dummdidumm
Copy link
Member

Good point - yes, we can close this now. Additionally knowing inside the component whether or not the property is bound is too niche of a use case to support. It's better to have separate props in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting submitter needs a reproduction, or clarification
Projects
None yet
Development

No branches or pull requests

6 participants