Function changing state in class does not trigger reactivity

[ecstrema]

Describe the bug

I can't seem to wrap my head around svelte's new reactivity system.

Below is a simple example, and I don't understand why it is not working. You just create a state class:

class Test {
  ready = $state(false);

	init() {
		this.ready = true
	}
}

export const test = new Test();

and import it in the other component.

<script>
	import {test} from "./test.svelte"
</script>

<h1>{test.ready}</h1>

<button onclick={test.init}>Click this to set the above to true</button>

But the UI doesn't update.

Reproduction

https://svelte.dev/playground/0d52f8525fbf4e4382eda285c426aec7?version=5.19.2

Logs

System Info

REPL

Severity

annoyance

[Conduitry]

You need to use onclick={() => test.init()}, otherwise this isn't bound correctly when calling the method.

[ecstrema]

Oh thanks.

So if this function comes from a library and I have no way to know if it is reactive, I need to do this too...

It seems hard, but wouldn't there be a way for the compiler to change all event={fx} to event={() => fx()} during development, and then issue a warning when it detects that it should be reactive?

[Conduitry]

You should be doing () => foo.bar() generally instead of foo.bar, regardless of whether reactivity is involved. foo.bar won't actually call the bar() method specifically on foo. If you look at something like this, where there's a setInterval() to output the value of test.ready every second, you'll see that it actually doesn't change when you click the button. Svelte's reactivity is not involved. This is just how calling methods in JS works. You can't do const method = object.method; console.log(method()); have expect it to be the same as console.log(object.method());.

[ecstrema]

Well even better then!

That means you could always issue a warning when someone puts just foo.bar instead of () => foo.bar()

[Prinzhorn]

See also https://stackoverflow.com/questions/56503531/what-is-a-good-way-to-automatically-bind-js-class-methods

https://svelte.dev/playground/0d52f8525fbf4e4382eda285c426aec7?version=5.19.2#H4sIAAAAAAAAA3WRTU_DMAyG_4qJOKzS1Ipr6Yq4cUbcCEJZcVkgdarEHZuq_neStMAm4Na-H9ZjZxSkOhSluEfVsN7jDTRGeS_WotUGvSgfR8HHPkaiEPSlcNv3ud-j4ahtlce_9MYSI3EYI6pZLG3P2pIHNxB6KGpJkirfON1z_Gbd9dYxjIyeJ2id7UCKvIi_y1wpQqP4qVC1u6rv0Bg7t3KH6uU4XVRF0JO_HZgtgaXG6OZ9M6c0aZ7qqFTFHKgDMeOBRcluwGn9z-YnKPmbP9_-l3dyATykzdJ94SEEYZQE8JxwYQOXnhXjqlXGY3YdwcM1AmSwVhls6hiXLJl32uffrYga9WkuvCJDskJlyTvkwRGc1JZ8qnxRhTdhiPhhJuFHAlxl5xd5mj4BrDx0pTACAAA=

https://svelte.dev/playground/0d52f8525fbf4e4382eda285c426aec7?version=5.19.2#H4sIAAAAAAAAA3WQzU7DMBCEX2WxOKRSlYhrSIO4cUbcMEKuu6WGdB3Zm9IqyrtjO6VN-bnZs_OtZ9wLUlsUpXhEpdns8A50o7wXc7E2DXpRPveCD220RCHoR-C-bXO_w4ajtlQe_9K1JUbisEZUo1jalo0lD64j9FDUkiRVXjvTcjyz2bbWMfSMngdYO7sFKfIiXo97pQhEcUao2tzUD9g0dqRyh2p1GK6qIuhpvuyYLYEl3Rj9sRhdhgwPdVSqYjTUITHjnkXJrsNh_k_zSZT83V-2_zWb_ADuU7P0v_AUjNBLAnhNcWEB154VY7ZWjcfZbQwuOeA-hNFsXTaLfsmSeWN8ih-g0zlfGlpl8ZpYHsYFcfSDPD0YW06sb8iQRme_Q-4cwQQ7-hPyXShmhNg87CT8TN2y2eVnvgxfmxMu7msCAAA=

That means you could always issue a warning when someone puts just foo.bar instead of () => foo.bar()

No, because if you don't rely on this it doesn't matter and you can use foo.bar just fine.

[Prinzhorn]

No, because if you don't rely on this it doesn't matter and you can use foo.bar just fine.

However, I do agree that in dev Svelte could inject code into all class methods that warn you that a method was called with the wrong context. I know that this is a JavaScript "feature" and people "should know", but Svelte could also make the dx here better. @Conduitry thoughts? Are there legitimate reasons that you'd want to call a class method with a different this? I doubt it, as this also breaks TypeScript assumptions.

[ecstrema]

No, because if you don't rely on this it doesn't matter and you can use foo.bar just fine.

A warning is not an error though. It's meant for best practices. I think if we can have strong a11y warnings, we can have strong warnings for dx too.

[ecstrema]

However, I do agree that in dev Svelte could inject code into all class methods that warn you that a method was called with the wrong context.

I don't think this should be in class methods. I think it should be at binding sites: replace

<button onclick={test.init}>Click this to set the above to true</button>

with

<button onclick={checkUnboundSubscriptions(test.init, loc)}>Click this to set the above to true</button>

and

function checkUnboundSubscriptions(fx, loc) {
	const prev = getSubscriptions()
	fx()
	if (!getSubsriptions() = prev) {
		issueWarning(`${loc} - a state susbbscription happened into an unbound class function. use event={() => fx()} instead of event={fx} to track reactive state. See also https://github.com/sveltejs/svelte/issues/15098#issuecomment-2610943014`)
	}
}

This is of course an overly simplified view, but I think this would save countless hours of developer time.