fix: spread as a dependency for every prop

[paoloricciuti]

Svelte 5 rewrite

Should fix #11286 the problem was that when you pass a spread and try to access a prop before the spread the code access the spread to check if the accessed prop is in there. This however register a dependency on the spread regardless if the key is in there or not. To fix this i've untracked the access of every function (this required re-accessing the function after the check if the key was in there...i don't know if there's a better way to do this).

There are some tests failing and i've yet to write the test for this so this is a draft.

Please note that the Svelte codebase is currently being rewritten for Svelte 5. Changes should target Svelte 5, which lives on the default branch (main).

If your PR concerns Svelte 4 (including updates to svelte.dev.docs), please ensure the base branch is svelte-4 and not main.

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5acbfce

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dummdidumm]

I don't think this will work - what if the dynamic bit doesn't contain the property initially, but after an update it does? Then it won't get notified anymore, leading to stale values

[paoloricciuti]

I don't think this will work - what if the dynamic bit doesn't contain the property initially, but after an update it does? Then it won't get notified anymore, leading to stale values

Yeah that's exactly the failing tests. I pushed the code because I can't work on it now and planned to continue working on it this evening (but at the same time just wanted to get the problem out there and the possible solution validated).

[paoloricciuti]

@dummdidumm could pass in a source with the keys of rest mitigate the issue? We could have a dependency solely on the keys on not on the whole object. But yeah the more I think the more it's clear that we need some sort of DEP on rest anyway

[paoloricciuti]

@dummdidumm could pass in a source with the keys of rest mitigate the issue? We could have a dependency solely on the keys on not on the whole object. But yeah the more I think the more it's clear that we need some sort of DEP on rest anyway

I've implemented this and it works (except for two legacy tests that i'm investigating):

1. to get the keys of the rest object it passes an array with empty spots for every prop except the spreaded ones. I changed the arguments of the spread_props function to also accept this.
2. In the function when a "function" is found it tries to get the same index in the keys array and eventually subscribe to the derived (this ensure that changes of shapes are tracked)
3. same as before the if the key is present the function is accessed again to subscribe to value changes

The thing the i dislike the most is that to get a reliable derived that doesn't change every time i had to Object.keys(rest).join(",") otherwise the array would be a new reference each time. The tests that are not passing regards a legacy code spreading into slot. I will try to investigate this a bit further tomorrow.

[paoloricciuti]

@dummdidumm i don't know if you can be of any help but the test that is breaking is this:

<script>
	export let a;
	export function b() {}
	export let c = 1;

	$: length = Object.keys($$restProps).length;
	$: values = Object.values($$restProps);
</script>
<div>Length: {length}</div>
<div>Values: {values.join(',')}</div>

<div {...$$restProps}></div>
<div {...$$props}></div>

ith the main.svelte be

<script>
	import App from './App.svelte';
	let a = 1, b = 2, c = 3, d = 4, e = 5;
	let f = { foo: 1 };

	function updateProps() {
		a = 31;
		b = 32;
	}
	function updateRest() {
		d = 34;
	}
	function updateSpread() {
		f.foo = 31;
	}
	function updateSpread2() {
		f.bar = 2;
	}
</script>

<App {a} {b} {c} {d} {e} {...f} />
<button on:click={updateProps}></button>
<button on:click={updateRest}></button>
<button on:click={updateSpread}></button>
<button on:click={updateSpread2}></button>

This basically breaks only when you click updateRest before updateSpread and i found out that it only breaks if i subscribe to the derived of the keys of the spread. I don't understand how an extra dependency prevent the firing of another dependency. Do you have any ideas?

[paoloricciuti]

Apparently #11364 was exactly what was causing the test to fail...updating the runtime locally gave me all green (after i fixed a bug with slot props).

So once #11364 is merged this is in theory ready. Still not convinced by the approach and open to a better one tho.

[paoloricciuti]

As expected this is now all greens

[paoloricciuti]

Also quick thing: I did not modify anything for server since we actually don't need to track the dependency there. This make the signature for spread_props different from server and client. I don't think It's a problem but just want to get this out there.