Actions and invalidation during update

[Conduitry]

Every Block's update function was written with the assumption that nothing in it would invalidate any part of the component's context, but this might not be the case when actions are used.

#4095 wasn't the correct solution to #4094. It was heavy-handed in that it removed all of the optimizations we could have when the entire dirty bitmask fits in one number. And it didn't actually fix the bug when the element that needs to be updated was located before the element that had the action attached to it.

Consider the examples (courtesy of @dkondrad and @tanhauhau) https://svelte.dev/repl/1fb88e85446143578289742e81fae65b?version=3.16.4 and https://svelte.dev/repl/5239a903de1b401f92113318dcb344cc?version=3.16.4 - The first one broke in 3.16.0 and then was fixed in 3.16.4 because mutations to the bitmask array are now affecting the rest of the update. The second one is still broken in 3.16.4 (and I think has always been broken) because the data is invalidated after we've already seen whether we should update it.

The obvious way to handle this is to make sure we schedule a new update if anything got invalidated during the update. One way to do that would be to make all calls to actions happen asynchronously, so that their updates happen afterwards. There might be a more elegant way to handle this.

I think we can also reinstate the destructing fairly easily and generate even better code than we were pre-3.16.4 in cases where there is overflow. The argument to the update function could always be an array destructuring. code-red should make it fairly easy to generate a bunch of local variables for these called #dirty0, #dirty1, etc., and we could just refer to those instead of subscripting the array. This should also let us (continue to) avoid having two cases depending on whether there's an overflow.

[Conduitry]

My last idea about always destructuring is proving to be messier than I hoped. It's probably not impossible to push through, but at this point I think it might be better to just revert #4095 and introduce some mechanism that causes an additional invalidation from whatever's changed by the actions.

[ghost]

@Conduitry,
I have an idea how to fix my repro case, but I'm still trying to sort out where in the compiler to make the change.

If you look at the compiled code, there is a second check against the bitmask after the action callback is triggered. These checks are inserted based on looking for assignments and member acceses in the function. So if you add another variable and set it in the callback, the appropriate recheck is inserted.

The problem is that whatever code is responsible for that doesn't check for method calls against stores.

Ideally, if we trap method calls to set and update and check if the object is also a store $name then we should just be able output another bitmask check and set_data like any other reactive var.

Is there a way to annotate the generated code to tell where in the compiler output comes from? I'm extremely familiar with complier construction, but there is a ton of ground to cover. Walking the ast in the debugger is also a grind.

[tanhauhau]

Is there a way to annotate the generated code to tell where in the compiler output comes from? I'm extremely familiar with complier construction, but there is a ton of ground to cover. Walking the ast in the debugger is also a grind.

@dkondrad my very unorganised way of doing it is to do a global search for some common keyword in the generated code from the compiler 🙈

[tanhauhau]

After the update function, svelte will clear the $dirty array.

so if we simply schedule another update when $invalidate is called within the p function, the $dirty may have been cleared.

so I would like to propose to have a $dirty_wip that will be used for $$invalidate to mark the dirty bits during p function.

so when the p function has finished, and we find that the $dirty_wip is dirty, we would assign it to the $dirty and schedule another update.

so the p function will use the snapshot of the dirty bit, that will not be mutated by the $$invalidate calls within the p function.

[ghost]

Another idea I had was to do a quick reduce on the dirty array to see if it has changed after the action callbacks, which I guess is basically what dirty_wip is. Since the mask is additive then all you'd have to do is sum them before running and sum them again afterwards.

If this were an embedded project, I'd just right shift dirty until there were no bits left instead of adding explicit test statements for every ctx in the mask.

@tanhauhau, I'm on a 13 hour time diff so I'll be turning in but will check back in the morning.

[Conduitry]

These changes were released in v3.16.5.