Skip to content

Explicit bindings with $props.bindable() #10768

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
Rich-Harris opened this issue Mar 12, 2024 · 29 comments · Fixed by #10851
Closed

Explicit bindings with $props.bindable() #10768

Rich-Harris opened this issue Mar 12, 2024 · 29 comments · Fixed by #10851
Milestone

Comments

@Rich-Harris
Copy link
Member

Rich-Harris commented Mar 12, 2024

Describe the problem

Today, every component prop is bindable. That means that if a component does let { foo } = $props(), a consumer can do bind:foo and observe changes to foo inside the component.

That's fine — bindings are a useful feature, and no-one has really complained about this — but it's not ideal:

  • well-written applications use bindings very sparingly — the vast majority of data flow should be top-down
  • as a component author, you have no control or visibility into how people will use your component. there is less predictability — you cannot rely on locality of behaviour
  • as a component user, it is hard to tell which things are intended to be bound — you are reliant on documentation
  • Svelte itself must assume that all props are bindable, which adds potential overhead

This issue is more salient in Svelte 5 than previously: because reactivity is based on signals rather than static analysis, mutating an object can have distant effects. We mitigate this by warning the developer (in dev mode) against mutating state owned by another component, but the way to prevent that warning is to pass props around with bind:. Unless we add a modicum of friction, the temptation will be to use bindings willy-nilly, which is the way of spaghetti.

Describe the proposed solution

We plan to introduce a new rune, $props.bindable(). Usage is identical to $props(), but all the identifiers declared within are considered bindable:

<script lang="ts">
  interface Props {
    type: string;
    value: string;
  }

  let { type }: Props = $props();
  let { value }: Props = $props.bindable();
</script>

A user of this component could do value={value} or bind:value={value}, but bind:type={type} would cause an error (both a type error and a runtime error).

Bindings would continue to exist in ...rest properties...

let { type, ...rest } = $props();
let { value } = $props.bindable();

rest.value; // exists! but is readonly

...but would be readonly, as they are today.

(We considered a variety of alternative designs — annotating prop interfaces with a Bindable type, adding an option to the $props(...) call, using defaults (let { value = $bind() } = ...) and so on, but they all had major downsides.)

Importance

nice to have

@AdrianGonz97
Copy link
Member

I love this! I've been hoping for some kind of explicit declaration of binds ever since I read this proposal a while back: #9997

With that said, to bikeshed on the name for a bit, similar to #10334, $props.bind() may not be the best name since it's something that already exists on the function prototype.

Perhaps some better alternatives could be $props.binds() or $props.bindable()?

@dummdidumm
Copy link
Member

dummdidumm commented Mar 12, 2024

lol what are the odds, prototype methods strike again.

$props.bindable() has my vote then, also more clearly says that this is maybe a binding, not that it must be a binding.

@Rich-Harris Rich-Harris changed the title Explicit bindings with $props.bind() Explicit bindings with $props.bindable() Mar 12, 2024
@Rich-Harris
Copy link
Member Author

Ha, whoops! Agree — I've updated the issue to $props.bindable()

@harrisi
Copy link

harrisi commented Mar 12, 2024

Bindable also is more correct, since it doesn't necessarily mean it is bound.

I'm curious how this will interact with #9764. If the issue was with complications around defaults for bound values, does that mean defaults for $props could come back?

@dummdidumm
Copy link
Member

This doesn't change the rules around defaults. You can still use them for normal properties. It only errors when you have a fallback value defined but then do bind:value={foo} and foo is undefined.

@harrisi
Copy link

harrisi commented Mar 12, 2024

Would a naming scheme using in/out/inout be beneficial? With this proposal, $props would be "out", $props.bindable would be "inout", but there is no "in" - you need to use $props.bindable and be trusted not to modify it.

@dummdidumm
Copy link
Member

If you're only interested in the value changing, use an event callback. Using bind:x to react to x changing means using an $effect, and that logic is better implemented using a callback prop.

@pngwn
Copy link
Member

pngwn commented Mar 12, 2024

If you're only interested in the value changing, use an event callback.

This assumes you are the author the component, for third party components, being able to observe prop changes is really nice for a variety of reasons. If an author doesn't make their prop bindable then you have to make a PR or you are done.

@dummdidumm
Copy link
Member

Can you give a concrete example? I've never seen this in the wild before

@GauBen
Copy link
Contributor

GauBen commented Mar 12, 2024

Very glad to see this happening!

@zhihengGet
Copy link

zhihengGet commented Mar 12, 2024

since we're doing this ,

can we do

props.readonly () : readonly props top-down runes

$props.writable() or props.bindable() : what rich said

$props(): combines above too , no breaking change for current svelte 5 people

in the doc , we can just say readonly and bindable are more recommended ... i would love to keep $props bindable, this would make not change any exisitng code

@Conduitry
Copy link
Member

We definitely don't want to optimize for Svelte 5 early adopters. It's much better to make current Svelte 5 users deal with a breaking change than it is to ship an API with 5.0 that we wish worked differently.

@zhihengGet
Copy link

zhihengGet commented Mar 12, 2024

We definitely don't want to optimize for Svelte 5 early adopters. It's much better to make current Svelte 5 users deal with a breaking change than it is to ship an API with 5.0 that we wish worked differently.

sure more work to do :( , but i still think $props.readonly is more explicit than $props alone, best of both worlds

if u really dont like what i propose then plz plz add some error message so i can migrate $props one by one

@Rich-Harris
Copy link
Member Author

As I said in the opening comment, using a binding with a non-bindable prop will cause both a type error and a runtime error

@harrisi
Copy link

harrisi commented Mar 12, 2024

Hm, I was confused by some behavior in #10779 (thanks, @Conduitry for explaining it) while thinking about this. Your example uses strings, but how would this work with objects? Something like this: repl.

As far as I understand, $props.bindable still won't (be able to(?)) enforce an immutable one-way binding for objects, correct?

@dummdidumm
Copy link
Member

During implementing this I came across some edge cases which made me question whether or not this is the best design for declaring properties as bindable. It basically all comes down to the fact that it's two runes instead of one.

Initially I though we make it a compiler error to do destructure the same property from both runes:

let { a: foo } = $props();
let { a: bar } = $props.bindable(); // error, cannot destructure in both

Turns out, we need to allow it because it would mean preventing people from doing something like this, which might be crucial to not introduce unwanted behavior:

let { value: _, ...rest } = $props(); // rest should not contain value
let { value } = $props.bindable();

More generally, it's a gotcha one might need to watch out for, that using a ...rest property does contain a variable from the other props rune (this could lead to real bugs!). For people creating component libraries that wrap elements this could be a rather common source of confusion, since they want to spread events, and so a ...rest property is inevitable

The other thing that doesn't feel quite right - to me at least - is that from a types perspective, it's best to declare the props type once and then share it:

type Props = { foo: string; bar: number }
let { foo }: Props = $props();
let { bar }: Props = $props.bindable();

I can't really articulate why, but this looks like everything is bindable, because the type before $props.bindable() tells me "these are the things you can expect from here - but in reality it's the actual destructured properties that define what is bindable. The alternative is to keep the types separate, but I think this is much worse since we're now back to having multiple types declare what a component API looks like.

Make it part of $props()

I'm therefore contemplating whether or not an alternative design is better: Keep the definition of what is bindable part of $props(). There are two variants to this (kudos to Dominic for the second variant).

$props({ bindable: [..] })

The first variant is to introduce an options object on $props() with a bindable property. You then list out all the properties that are bindable:

type Props = { foo: string; bar: number; baz: boolean }
let { foo, bar } = $props({ bindable: ['foo', 'bar'] });

As a shortcut you can set bindable: true to allow all properties to be bindable.

$props().bindable(..)

The second variant is to call bindable() on the return of $props(). You then list out all the properties that are bindable:

type Props = { foo: string; bar: number }
let { foo, bar } = $props().bindable('foo', 'bar'); // bindable takes 0-n arguments

As a shortcut you can call bindable() without any arguments to allow all properties to be bindable.

In both variants, Svelte language tools would make sure that you can only add props that appear in the props type definition (if such a definition exists).

Advantages over $props() / $props.bindable()

  • keep one rune from which you get everything: clearer where to look, more consistent to type (don't have to decide between multiple ways of how to do it)
  • prevents edge cases around destructuring the same variable twice, and rest props

Disadvantages to $props() / $props.bindable()

  • we need to decide whether the syntax of $props().bindable(..) / $props({ bindable: [..] }) should be restricted to string literals or not.
    • If is restricted, it possibly means more to type, but more predictable and better analyzable for future use
    • If it isn't restricted to string literals you can theoretically end up in a place where TypeScript within Svelte language tools cannot infer which things are bindable, and so it falls back to marking everything as bindable. This can only happen if the input is dynamic and the variables are typed as string and not as the specific literals they are (e.g. $props({ bindable: foo ? [x] : [...y] }) and x is just of type string). Note that the runtime validation would still catch mistakes!
  • you write it out the name of the binding one more time (though autocompletion will help you here, and in general you don't have many bindings, if at all, on components)
  • something else?

Would love to get some opinions on this. You can also just thumbs up (agree for the change) or thumbs down (keep the initial proposal) on this comment.

@Rich-Harris
Copy link
Member Author

My objections to $props({ bindable: [...] }) and $props().bindable(...) are partly aesthetic, partly technical.

On an aesthetic level, using strings like 'foo' to describe characteristics of free variables like foo feels jarring to me. I guess you could make the case that it's referring to the 'foo' property of the props object — props['foo'] — but since that object is something Svelte (rather than the component consumer) creates, it sort of feels like mixing API with implementation details. It irks me in a way I can't fully articulate.

It also feels less structured. $props() and $props.bindable() says 'here are the one way props, and here are the bindable props'. There's a meaningful distinction between them that is reflected as you read the component code. Passing strings to a function says 'here are my props, and oh by the way some of the things you were looking at back up there behave differently to anything not mentioned down here'. There's no requirement for things to be grouped in any sensible fashion, and so you have to read more stuff to gain a complete picture.

Finally, it feels less consistent with the way we do things elsewhere:

  • $effect.pre(fn) rather than $effect(fn, { mode: 'pre' })
  • $state.frozen(...) rather than $state(..., { frozen: true })
  • $derived.by(fn) rather than $derived(fn, { fn: true })

The technical objections relate to this question:

we need to decide whether the syntax of $props().bindable(..) / $props({ bindable: [..] }) should be restricted to string literals or not.

Either answer is deeply weird:

  • if it's syntactically restricted, it feels very uncanny-valley-like — ordinarily anything that gets passed to a rune is Just JavaScript ($effect functions, $state and $derived expressions etc) but suddenly we have a case where it's not Just JavaScript. On discovering that this doesn't work...

    import { bindable } from './elsewhere';
    let { x, y, z } = $props({ bindable });

    ...I would immediately start wondering what other restrictions existed in Svelte. Suddenly it doesn't feel like I can reason as confidently about the system as I could before.

  • if it's not syntactically restricted, then the feature gets less useful — since we can no longer guarantee that we know at compile time which properties are bindable, we lose typechecking guarantees (one of the main motivations for doing this in the first place) and whatever unanticipated opportunities arise in future (such as automated docs/storybook generation, idk) from having that information available to the compiler.

Either way — syntactically restricted or not — we face the possibility of future extensions to $props() having the opposite requirement. I really think it's best to sidestep this question altogether.

@98mux
Copy link
Contributor

98mux commented Mar 18, 2024

export let bindable:number = 0
export const notBindable:number = 0

😄

@abdel-17
Copy link

Maybe $props() can just omit the props in $props.bindable(). The language tools can infer the types returned from a common exported type like Svelte 4's $$Props.

type Props = { foo: string; bar: string; }

let { foo } = $props.bindable();
let { ...props } = $props();  // { bar: string }

@mquandalle
Copy link

How about having $props() expose only non-bindable props, and then a new rune $props.all() for all props, including bindable props in read-only mode.

That would make the examples of @dummdidumm work. Here it's possible to raise an error:

let { a: foo } = $props();
let { a: bar } = $props.bindable(); // error, cannot destructure in both

But with $props.all() there is no error, the prop a can be accessed from both runes:

let { a: foo } = $props.all();
let { a: bar } = $props.bindable(); //no-error

// foo is read-only

@Azarattum
Copy link
Contributor

Azarattum commented Mar 19, 2024

Hmm... What is exactly wrong with?

let { a, b = $state(), c = $state(42) } = $props();

where b, c are bindable

  • no new runes
  • states indicates that the value is mutable
  • we still can pass a fallback value
  • if anything, it feels more like extending the syntax rather than constraining it (one more place where $state is valid)

@Rich-Harris, you have mentioned that a similar approach have been considered and had downsides. Could you elaborate what are those?

@wbhob
Copy link
Contributor

wbhob commented Mar 19, 2024

Svelte's whole thing is syntax sugar and using the compiler to help you, why not have a special label for bindable props? # is the label for private in classes, so an inverse example could be let { #unbound, bound } = $props(). But obviously that's not ideal since being bindable is not default. Perhaps labelling bound variables with $? let { unbound, $bound } = $props() and the component would just accept bind:bound={someState}

Alternatively, using colon to indicate it's bound.

let { unbound, :bound } = $props()
let { unbound: foo, :bound: bar } = $props()

@7nik
Copy link
Contributor

7nik commented Mar 19, 2024

@wbhob, the syntax must be a valid JS/TS, so it's no-go.

Talking about $props().bindable(..), doesn't $props() must return an object with the bindable field, which may cause type conflict if somebody tries to define a bindable prop?

let { bindable }: { bindable: boolean } = $props();

I prefer the single-rune solution, though I agree with Harris' objections. I wonder what are the cases with the dynamic list of bindable props.

@harrisi
Copy link

harrisi commented Mar 19, 2024

One thing that might be worth noting is that, while Svelte components aren't web components, web components have a special static property named observedAttributes which must be an array of strings. While this is not exactly the same situation, it's similar enough and people may be familiar. So here are some things I thought of while thinking about that.

  1. A special prop that you assign to. Something like this: const $props.bindable = ['foo', 'bar'].
  2. Utilize class expressions and static initialization blocks: something like this.
  3. Slightly related - a separate <script> context - <script context=props>. Alternatively, option one could only be available in <script context=module>.

I don't think any of these ideas are ideal, but maybe they'll help figure out a better idea. I'll also just add that I think having an object passed to runes for configuration would be bad - unless the other runes were changed to not be "properties" of runes and use objects themselves. But I don't like that option either, really.

EDIT: fixed the repl link - not sure what happened.

@7nik
Copy link
Contributor

7nik commented Mar 19, 2024

1 suffers the same problems as $ props () .bindable (...). Plus, it's a weird way to configure behaviour for people not familiar with custom elements, and I'm sure more people are familiar with another framework than with custom elements.
2 and 3 are over-engineering.


What about

let { 
  foo, 
  bar,
  bindable: {
    value,
    baz,
    ...bindableRest,
  },
} = $props();

Syntactically restricted, no configs, no renaming problem.
But then the bindable prop becomes reserved. And readonly if it will be introduced. Though, the $bindable and $readonly keys can be used to reduce this problem and highlight that it's syntax but not a prop.
Edit: though it suffers from the ability to declare the same prop twice and is tricky to type.

@wbhob
Copy link
Contributor

wbhob commented Mar 19, 2024

While my solution may not be ideal, I think labelling the prop in some way in its primary declaration will be more effective than redeclaring it or declaring it separately

@98mux
Copy link
Contributor

98mux commented Mar 19, 2024

Lets look at how the typing complexity is, by removing all letters

let { foo }: {foo:string} = $props();
let { bar }: {bar:number} = $props.bindable();

Becomes

{}:{:}=$();
{}:{:}=$.();

A crazy total of 23 special characters! And a total of 9 unique characters ({}:=$();.). Wonder how the vibes on that typing is! Typing that would be like doing a dance on the keyboard, but I mean, dancing is a good vibe.

export let bindable:number = 0;
export const notBindable:number = 0;

becomes

:=;
:=;

A total of 6 special characters! 3 unique character (:=;)

@abdel-17
Copy link

export let bindable:number = 0;
export const notBindable:number = 0;

A total of 6 special characters! 3 unique character (:=;)

Except you have to repeat export let and export const for every prop, and you can't access rest props without the special $$restProps variable. And if you want it to be type-safe, you have to explicitly set the $$Props type, and now you're managing types twice for each prop and it just becomes a mess.

@Gin-Quin
Copy link

Gin-Quin commented Apr 17, 2024

Hard to find the best workaround when you're facing the language limitations.

There is clearly a need for decorators here (in a destructured assignment):

let {
  cantBind = "",
  @bindable canBind,
}: {
  cantBind: string,
  canBind: string
} = $props()

That's what decorators are for: labelling properties.

Also, if Typescript would allow type definitions right in the object, that would help a lot of frontend frameworks to keep dry:

let {
  cantBind: string = "",
  @bindable canBind: string,
} = $props()

Which is the theoretical best syntax for declaring props.

Interestingly, Flow recently implemented the second syntax.

Good job guys finding your way out of this maze. The language designs are not helping you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet