svelte icon indicating copy to clipboard operation
svelte copied to clipboard

`bind:clientWidth={w}` updates `w` too late

Open Rich-Harris opened this issue 1 year ago • 4 comments

Describe the bug

Given code like this...

<script>
  let w = 0;
  let h = 0;
  let div;

  $: {
    console.log(!!div, w, h);
  }
</script>

<div bind:this={div} bind:clientWidth={w} bind:clientHeight={h} class="box"></div>

<style>
  .box {
    width: 100px;
    height: 100px;
    background: #ff3e00;
  }
</style>

...Svelte 4 will log the following:

false 0 0
true 100 100

Svelte 5 will log this instead:

false 0 0
true 0 0
true 100 100

Using $state and $effect, we get this result:

true 0 0
false 100 100

That true 0 0, when the element binding exists but width/height bindings haven't updated, causes problems — see for example the Svelte 5 preview site using Svelte 5, which borks up the split panes because of this code:

// constrain position
$: if (container) {
  const size = type === 'horizontal' ? w : h;
  position = constrain(container, size, min, max, position, priority);
}

Reproduction

See above

Logs

No response

System Info

`next`

Severity

annoyance

Rich-Harris avatar Feb 17 '24 20:02 Rich-Harris

Investigated this a bit. I think we can fix this by replacing this code...

https://github.com/sveltejs/svelte/blob/df10204fcc8b6ebb19f3589590d257d4352fe0b2/packages/svelte/src/internal/client/render.js#L948-L954

...with a user_effect instead of the brittle-looking requestAnimationFrame stuff:

export function bind_element_size(dom, type, update) {
  const unsubscribe = resize_observer_border_box.observe(dom, () => update(dom[type]));
  user_effect(() => {
    untrack(() => update(dom[type]));
    return unsubscribe;
  });
}

If we do the same for bind_this, everything will be in sync.

The wrinkle is that these effects need to happen before actual user effects.

Rich-Harris avatar Feb 17 '24 21:02 Rich-Harris

Yeah the problem here and with bind_this is that the elements aren't added to the document synchronously as before, so you need to wait asynchronously before all document fragments are attached, and then do the work, but as you say in a way that they happen before other user effects.

dummdidumm avatar Feb 17 '24 22:02 dummdidumm

So the easiest way to fix this without introducing yet another event type would be to move the bind_this and bind_element_size calls above user code.

We can't do that right now, because bind_this takes a third argument which would not have been defined by that point. However, there's a subtle bug in our bind_this handling, and so I think it might be easiest to rework that function a bit. Will look into this

Rich-Harris avatar Feb 18 '24 14:02 Rich-Harris

Another possibility to not introduce another effect type could be to instead go through the list of effects, find the first at the depth of the binding and insert before that (using splice).

dummdidumm avatar Feb 18 '24 15:02 dummdidumm