Do not null blocks on await outros

[vlasy]

Description

Do not null blocks after out transition in await block.

Motivation

If the then block has out transition, it takes some time for it to "disappear". After the update of pending block, then block is being transitioned out. If the promise is then resolved, then block is updated and pending block is transitioned out and then block set as current. Only after that, the then's transition out (triggered by the update of pending block) finishes and the corresponding block info is being nulled.

That results in a situation where await block is in then state, but then entry in blocks field is null. Therefore it is not transitioned out during the next pending block update.

Caveats

I found the original commit which introduced nulling out the transitioned blocks. It's almost 2 years ago by @Rich-Harris in 1d3b1284eaecf2d09557d883b0db3adbab48f633 but I don't know what was the original motivation, so I am afraid this change/revert might break something :(

Related issue

Fixes #1591

Tests

• All the tests are passing.
• I tried to write a test, but was not successful.

[tanhauhau]

I tried to write a test, but was not successful.

may i know what is the blocker on the test case?

[vlasy]

I tried to write a test, but was not successful.

may i know what is the blocker on the test case?

I wanted the test case to click on the element, wait for it to fade out, then click again, wait again, and then check the amount of elements. But the waiting part seemed to be the problem

[buhrmi]

I've tried to come up with a minimal failing testcase. Here is a link to the REPL where you can see the bug. I ran it with the patch applied but it still fails.

[buhrmi]

vlasy#1

[buhrmi]

Okay, I managed to come up with a testcase that fails on master, but will be fixed with the patch. https://github.com/vlasy/svelte/pull/1/files

Not so sure about the use of setTimeout in the testcase, but this bug was really difficult to make appear inside of a testcase.

Anyway, would be cool if it could be merged, because this bug is annoying me a lot.

[quasarchimaere]

is there any eta for this bug to be fixed in any future release? we are encountering the same issue when using sveltestrap components within await blocks, would be nice to know the progress for this in order to determine if we should find a workaround or not

[quasarchimaere]

it seems to me that quite a lot of people have problems with transitions within await blocks that cause dom node duplications, my initial comment was almost 2 months ago, simply asking for an eta on this, or at least for a workaround of some sort, unfortunately we aren't really in the position to contribute to this opensource project as a reviewing instance so we can't merge anything, but what i am trying to say is that it would be good to at least give feedback if anything is still missing that needs to be added to this MR, as the underlying issue #1591 seems to be fairly old though i am kind of losing hope that this will be fixed anytime soon...

[antony]

Hi @quasarchimaere - I apologise that we have not given feedback on this issue.

In terms of getting it merged, it would be difficult to merge this without a unit test of some sort, since without it, it would be subject to being broken again fairly quickly without anyone noticing.

In terms of providing an ETA, I'm not sure what sort of guarantees you would expect from a completely volunteer run open source project, but as I'm sure you can ascertain from where I'm going with this, we don't have the ability to provide any.

I'll self-assign this so that I can look to test it out at some point but I can't make any promises as to the when.

[buhrmi]

@antony I have provided a (admittedly rather clumsy) test case: https://github.com/vlasy/svelte/pull/1/files. Maybe it helps

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[quasarchimaere]

afaik this is still an issue and should not go stale

[quasarchimaere]

its somewhat frustrating that this PR seems to be unmergable due to the simple fact of it not providing a unit test.

yes the lack of (unit) test would make this issue prone to happen again fairly soon, but the solution to this can't be that the issue itself stays in the code for years now.

[benmccann]

@vlasy I know it's been awhile, but would you be able to rebase this PR?

[vlasy]

@vlasy I know it's been awhile, but would you be able to rebase this PR?

Sorry, but I can't

[dummdidumm]

The bug this fixes is no longer reproducible, therefore closing - thank you!