Over-permissive logic for .innerHTML optimizations

[Conduitry]

See https://github.com/simonlayfield/svelte-each-index - Svelte is using the .innerHTML optimization when it shouldn't. The {i}s in the loop should be dynamic.

As of #1499 this won't be reproducible in dev mode / in the repl.

cc @simonlayfield

[Conduitry]

Reduced example:

{#each foo as bar, i}
	<div><div title={i}>x</div></div>
{/each}

<script>
	export default {
		data: () => ({ foo: [0, 1, 2, 3, 4] }),
	};
</script>

So far it seems that the relevant factors seem to be: if we have an each loop, and we are only using its key (not its value), and that key is only being used in an attribute value, and the element with that attribute is nested inside another element, then Svelte incorrectly believes it is able to use .innerHTML. If I change any one of those initial conditions, the bug does not manifest.

[Conduitry]

To give a bit more of an idea of when this broke, it looks like it's fine in 2.1.1 but not in 2.2.0, meaning it happened in the big refactor in #1367, which is not incredibly helpful.

[Conduitry]

There's a bit more than this going on, but I think much of it can be traced back to here. In non-keyed each blocks, the index variable is declared as having no dependencies, and so stuff containing the index variable isn't seen as dynamic. Getting rid of the this.key check and always using the expression's dependencies does seem to resolve this bug, but it doesn't quite feel like the correct way. Perhaps there needs to be more nuance between the concepts of 'this expression doesn't depend on any other expressions for the purposes of recalculation' and 'this element isn't dynamic and is therefore safe to insert with .innerHTML'.

[Conduitry]

Okay one last thing before bed: I'm now pretty sure it's that last thing about needing more nuance. This bug isn't even particularly related to each blocks. A very simple thing which broke between 2.1.1 and 2.2.0 (and which can be confirmed in the REPL) is

<div><div title={'hello'}>x</div></div>

The compiler (reasonably and correctly) sees that 'hello' has no dependencies, and doesn't record any. But then it erroneously uses this lack of dependencies to deduce whether it's safe to serialize this to regular old .innerHTML. We need to be also tracking some additional, more basic, kind of thing when traversing the AST ('is it dynamic?'), which will let us decide whether we can use .innerHTML.

Edit: Okay it might not be all that complicated. I'm hoping that the only place this will affect us is in attribute values and that we'll just be able to check whether any of them have mustache expressions - but this should be investigated. I'll look into it tomorrow.

[Conduitry]

Didn't mean to autoclose this until the fix is actually released.

[Conduitry]

Fixed in 2.9.2.