[v2] Fix issue where class directives wouldn't work with spread props and class prop

[umanghome]

Currently, if there is any spread prop along with a dynamic class prop on the element, the class: directives stop working once the value of the variable that class was dependent on changes. This is because it sets the class prop of the DOM element directly, removed the class attached by the class: directive. And since the variable affecting the directive hasn't changed, a class for it is not applied again.

This should not happen, and class: directive should work just fine with spread props.

I have added tests in this PR as well, which were failing with v2.16.1, but are fixed with the succeeding commits.

Can't save a v2 REPL anymore, so adding an example in the PR description itself.

WithoutSpread.svelte

<div
  class={allClasses}
  class:reverse
>
  <slot></slot>
</div>

<script>
  export default {
    data: () => ({
      classes: [],
      reverse: false,
    }),

    computed: {
      allClasses: ({ classes }) => {
        const defaultClasses = ['without-spread'];

        return defaultClasses.concat(classes).join(' ');
      }
    }
  }
</script>

withSpread.svelte

<div
  class={allClasses}
  class:reverse
  {...attributes}
>
  <slot></slot>
</div>

<script>
  export default {
    data: () => ({
      classes: [],
      reverse: false,
    }),

    computed: {
      allClasses: ({ classes }) => {
        const defaultClasses = ['with-spread'];

        return defaultClasses.concat(classes).join(' ');
      }
    }
  }
</script>

App.svelte

<div>
	<input type="checkbox" id="round" bind:checked="round" />
	<label for="round">Round</label>
</div>

<WithoutSpread
	{classes}
	{reverse}
>
	Without Spread
</WithoutSpread>

<WithSpread
	{reverse}
	{classes}
	attributes={{
		'aria-label': 'With Spread'
	}}
>
	With Spread
</WithSpread>

<script>

export default {
	components: {
		WithoutSpread: './WithoutSpread.svelte',
		WithSpread: './WithSpread.svelte',
	},

	data: () => ({
		reverse: true,
	}),

	computed: {
		classes: ({ round }) => {
			const allClasses = [];

			if (round) {
				allClasses.push('round');
			}

			return allClasses;
		}
	},
}

</script>

<style>
	label, input {
		display: inline-block;
		vertical-align: top;
	}
</style>

The generated update functions, with version 2.16.1:

WithoutSpread.svelte

p: function update(changed, ctx) {
	if (changed.allClasses) {
		div.className = ctx.allClasses;
	}

	if ((changed.allClasses || changed.reverse)) {
		toggleClass(div, "reverse", ctx.reverse);
	}
}

WithSpread.svelte

p: function update(changed, ctx) {
	setAttributes(div, getSpreadUpdate(div_levels, [
		(changed.allClasses) && { class: ctx.allClasses },
		(changed.attributes) && ctx.attributes
	]));

	if (changed.reverse) {
		toggleClass(div, "reverse", ctx.reverse);
	}
}

The generated update functions, with changes in this PR:

WithoutSpread.svelte

p: function update(changed, ctx) {
	if (changed.allClasses) {
		div.className = ctx.allClasses;
	}

	if ((changed.allClasses || changed.reverse)) {
		toggleClass(div, "reverse", ctx.reverse);
	}
}

WithSpread.svelte

p: function update(changed, ctx) {
	setAttributes(div, getSpreadUpdate(div_levels, [
		(changed.allClasses) && { class: ctx.allClasses },
		(changed.attributes) && ctx.attributes
	]));

	if ((changed.allClasses || changed.reverse)) {
		toggleClass(div, "reverse", ctx.reverse);
	}
}

[umanghome]

This issue might fix #2707, but only for v2 and not for v3. Once this PR gets merged and the v2 bug is fixed, someone can pick up similar changes for v3.

[Conduitry]

At this point, I don't think we're really in a position to start porting specific fixes back to v2. #2707 was but one of a host of buggy edge cases around attribute spreads that were probably all present in v2 as well. If someone wants to maintain an unofficial 'v2 with backports' fork, they are of course free to do so.