sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.4k stars 4.11k forks source link

Runes mode: cannot send data from child to parent #12905

Closed rChaoz closed 1 month ago

rChaoz commented 1 month ago

Current state

In Svelte 4 (and Svelte 5 without runes mode), it is very easy to unidirectionally send data from a child to a parent, both static/dynamic data, using props/constants:

<!-- Parent -->
<script>
  let value
</script>

<Child bind:value />

{value}
<!-- Child -->
<script>
  export let value = 123
  // or
  export const value = 123
</script>
<!-- on click: value++ -->

REPL: v4 | v5

The problem

Trying to do the same thing with runes activated in the Child component doesn't work. Trying to bind to an export const fails with the error bind_invalid_export, which then explains it's disallowed to bind to a const. Trying to bind to a $bindable prop instead fails with the error:

props_invalid_value
Cannot do `bind:value={undefined}` when `value` has a fallback value

So, it's become impossible to easily send a value from a child to a parent.

Alternative

Use bind:this , which is harder to use, especially with dynamic values, and more verbose/uglier when you need just one value:

<!-- Parent -->
<script>
  let child
</script>

<Child bind:this={child} />

{child?.staticValue}
{child?.dynamicValue?.value}
<!-- Child -->
<script>
  export const staticValue = 100
  export const dynamicValue = $state({ value: 123 })
</script>
<!-- on click: dynamicValue.value++ -->

Use case

My current use case is something like:

<script>
  let instances = []
</script>

{#each someArray as value, i}
  <Child {value} bind:instance={instances[i]} />
{/each}

The child component has an instance of a third-party library class that attaches to a DOM element, like so:

<script>
  export const instance = new LibraryClass(...)
  let elem
  onMount(() => {
    instance.init(elem)
    return () => instance.destroy()
  })
</script>

<div bind:this={elem}></div>

Proposed solution

Either re-allow the previous behaviour of binding to constants or to props with a default without passing a value, or introduce a directive that is specifically for passing data from a child to a parent, like get:childValue={value}. It should be documented that any changes to value will not be passed to the child and will be overwritten whenever the child emits a new value, and maybe the compiler can even emit errors/warnings when value is mutated directly, but I think it can get difficult to do so (e.g. get:childValue={some.deep[0].state}). The get: solution I think makes more sense since bind is supposed to be bi-directional.

Severity

annoyance

dummdidumm commented 1 month ago

This change was a deliberate decision to avoid hard to reason about code and to prevent SSR inconsistencies and performance degradations when they need to rerun when a child changes a parent. We're not going to change that, therefore closing.

Your use case can be solved through a callback function.

Parent:

<script>
  let instances = []
</script>

{#each someArray as value, i}
  <Child {value} bindInstance={instance => instances[i] = instance} />
{/each}

Child:

<script>
  export let bindInstance;
  bindInstance(new LibraryClass(...));
  let elem
  onMount(() => {
    instance.init(elem)
    return () => instance.destroy()
  })
</script>

<div bind:this={elem}></div>
rChaoz commented 1 month ago

Thanks for the solution, I did not realise it was that simple to do it with callbacks. Still, it would feel more "svelte" if it wasn't with callbacks :p whatever that means

arackaf commented 3 weeks ago

imo this is a big change from v4 that should probably be documented more clearly. It appears unidirectionally binding a value from child to parent is no longer supported? At least not if the bindable prop has a default value it should be seeded with?

It looks like the hidden solution is to set that default value in the onMount handler, rather than in the $bindable() arg?

rChaoz commented 3 weeks ago

Another quick thought of this: if bind:prop={value} is now forced to be bidirectional, isn't bind:this itself going against this own rule? It's a bit confusing that you can't pass undefined to bind:anything, but you have to pass undefined to bind:this?

@dummdidumm could you consider re-opening this at least for discussion/documentation?

Also, @arackaf, I think it's even simpler than that (REPL):

<script>
    let { prop = $bindable() } = $props()
    prop = 123
</script>
arackaf commented 3 weeks ago
<script>
    let { prop = $bindable() } = $props()
    prop = 123
</script>

ah genius - right this isn't React that's fine :D

Just needs to be documented a bit better

arackaf commented 3 weeks ago

I feel like maybe there's an api missing. It seems like an api smell setting a prop to be $bindable, and then having to immediately set it to whatever value it's SUPPOSED to be because $bindable can't take a fallback if you pass in undefined, and the fallback will be ignored if you don't pass undefined.

child-bind:value={foo}

? I dunno

MotionlessTrain commented 3 weeks ago

I thought it is possible to bind: to values exported using export const as well. Maybe something like that could help (they aren't props anymore, in that case)

arackaf commented 3 weeks ago

export const is svelte 4 - we're talking about Svelte 5 here

MotionlessTrain commented 3 weeks ago

Export const and export function are still available in svelte 5 Export let is not available anymore in runes mode

arackaf commented 3 weeks ago

Appears to be disallowed in Runes mode

image
MotionlessTrain commented 3 weeks ago

Sorry, I thought that only exported functions needed to be accessed that way

arackaf commented 3 weeks ago

Thinking about this further, if the way to make

<script>
    let { prop = $bindable("value") } = $props()
</script>

work, if prop is passed in as undefined (since it's expecting to be bound) is to simply do the transform to

<script>
    let { prop = $bindable() } = $props()
    prop = "value"
</script>

... can't the Svelte compiler make that transformation for you? bind feels like a really leaky abstraction with this limitation.