Fix use-directives called multiple times

[christianheine]

Fixes #4693.

Additional remarks:

As far as I can tell, the fix seems to be working fine. I added a new test which covers the problem (thanks @pushkine !) and also thoroughly tested this version in a rather large scale app.

But, as already discussed in the issue, I think @pushkine has a valid point that this should be considered more of a a short-term remedy to address the (potentially huge) memory leak caused by mounting directives over and over. A more thorough approach would be in order in the long run.

[nicolodavis]

@christianheine The fix doesn't seem to be working for me. Perhaps I'm testing it incorrectly or maybe misunderstanding the scope of the fix.

Here is the relevant section of my application code (SVG):

  <g transform="translate({$viewportX}, {$viewportY})">
    {#each $renderingOrder as id (id)}
      <GameObject {id} />
    {/each}
  </g>

// GameObject.svelte

<g
  {id}
  use:drag={{ svg, snapshot: { x, y } }}>
  <svelte:component this={component} {position} />
</g>

Observation

use:drag is called when the list is reordered. Does this fix only ensure that the directive is cleaned up when it is invoked again (I notice that destroy() is called properly unlike the released version of Svelte)?

[pushkine]

use:drag is called when the list is reordered. Does this fix only ensure that the directive is cleaned up when it is invoked again (I notice that destroy() is called properly unlike the released version of Svelte)?

Yes it is a temporary fix that calls destroy() so as to at least avoid the memory leak, I said in #4693 that I'd look into a more complete fix last week but I haven't gotten around doing it yet

[nicolodavis]

Understood.

For the record, here is what breaks in my application. I'm implementing drag-n-drop via a use directive:

1. mousedown: attach mousemove and mouseup handlers.
2. mousemove: drag element
3. mouseup: cancel

I also "raise" the element in [1] by moving it to the end of the rendering order. This is necessary in SVG because of the lack of a "z-index" equivalent. However, this causes the directive to rerun, therefore losing the mousemove and mouseup handlers. The effect is that you can click on an element, but it does not drag on mousemove.

Perhaps this is not a good pattern to begin with irrespective of the bug in use directives?

[pushkine]

No you're good @nicolodavis, the problem comes from svelte
the fix could be as simple as this pushkine@3c8b618 though

[tanhauhau]

the fix could be as simple as this pushkine/svelte@3c8b618 though

i think you are right @pushkine, that looks like a good fix for now.

[Rich-Harris]

@christianheine thank you for documenting this bug and submitting a fix. I was a little unsure about passing #remount around everywhere — as it means every component gets slightly bigger rather than just the affected ones — so I opened a companion PR, #4860. The tl;dr is I think we can get rid of #remount altogether.