Don't directly reference local state

[dberlin]

In svelte 5, $state (here, the variable named 'state') referenced locally is not reactive, because of how evaluation occurs[1].
This means in the listed code, ...state will only ever refer to the original value of state, even if it is later updated or re-evaluated (by updateOptions, for example)

To make it reactive locally, the docs recommend using a closure (you can use getter functions and properties too if you want), which is what we do here.

This makes it so ...state always refers to the current value of state, even after updateOptions changes it..

This in turn, fixes the browser warning issued by svelte on all usage of createTable.

See https://svelte.dev/docs/svelte/compiler-warnings#state_referenced_locally for more details.

Svelte (currently) only warns if you do this and also reassign the variable, under the assumption that doing this means you want the local reference to see the new value. A cursory read of the code suggests we do want to see the new value post-updateOptions call, which means we need to lazily evaluate (or use a data class)

[1] The short, mostly correct version: The code is evaluated once, and $state/etc get transformed into function calls and proxies so that it dynamically re-evaluated when dependencies change. As a result, referencing state directly in the local scope (like this code does) only ever refers to the original value, even when you reassign it (as updateOptions does)

This is one of the more confusing foibles of svelte5, and has led to lots of complaining :)

[KevinVandy]

@walker-tx

[walker-tx]

Hi @dberlin - thank you very much for submitting a PR, and for the thoughtful technical explanation of the changes!

I don't see an issue attached to this, so I just want to confirm: Is the existing implementation causing an error of some sort? Eg: Loss of functionality, or a runtime/build error to be omitted?

Additionally, would using the mergeObjects utility function work here in lieu of wrapping state in a closure? I think it may have been done this way in the past. I've submitted this as a suggestion comment.

Thank you again!

[walker-tx]

Suggested change

	state: { ...getState(), ...tableOptions.state },
	state: mergeObjects(state, tableOptions.state || {}),

[dberlin]

Hey,

Hi @dberlin - thank you very much for submitting a PR, and for the thoughtful technical explanation of the changes!

I don't see an issue attached to this, so I just want to confirm: Is the existing implementation causing an error of some sort? Eg: Loss of functionality, or a runtime/build error to be omitted?

Additionally, would using the mergeObjects utility function work here in lieu of wrapping state in a closure? I think it may have been done this way in the past. I've submitted this as a suggestion comment.

Thank you again!

So this was discovered because I use newer versions of svelte/vite-plugin-svelte packages than y'all currently test against in the examples.

Since y'all distribute as svelte componenets (which is the right thing for sure), you get the user's version, not your version.

Once you upgrade to the latest vite-plugin-svelte and svelte for the examples it will issue this warning for every single example.
The warning is correct, they just changed it to be noticed in more cases and vite-plugin-svelte surfaces it properly now.

It pops up immediately (here's an example of running pnpm dev in examples/svelte/basic after updating the various packages in the repo):

  VITE v5.4.14  ready in 203 ms

  ➜  Local:   http://localhost:5173/
  ➜  Network: use --host to expose
  ➜  press h + enter to show help
6:51:46 PM [vite-plugin-svelte] /Users/dannyb/sources/tanstack-table-alpha/packages/svelte-table/dist/createTable.svelte.js:7:20 State referenced in its own scope will never update. Did you mean to reference it inside a closure?
https://svelte.dev/e/state_referenced_locally

I didn't bother to file an issue, but i can if it's useful - i'm positive I can break it using updateOptions.

Unfortunately, your suggestion won't work, it still references state directly. and by "won't work" i mean "won't reference the correct version of state. I don't know enough about the surrounding options/etc to know if it matters. So let me stick with that definition of "won't work" for the rest of this response, and you are free to tell me "cool, but doesn't matter for reasons x,y, and z".

Also, your fix would have worked in svelte4, but doesn't in svelte5.
In fact, my fix is buggy too, as writing this out convinced me of. I give the proper fix at the end.

I also apologize for the length of this, and if you know all this. I tend to think sometimes it's just best to walk these things through and see whether we end up with the same thoughts or not :)

So let me paste the post-svelte-compiled code for createTable, it's probably easier to just walk through it to understand what is going on. I hacked it up a bit to clean it up and highlight the parts we care about, but the whole thing is here if you want to see it. It's not too bad.

let state = $.state($.proxy(getInitialTableState(_features, tableOptions.initialState)));

const statefulOptions = mergeObjects(tableOptions, {
	state: {
		...$.get(state)
	}
});


function updateOptions() {
	return mergeObjects(prev, tableOptions, {
		state: mergeObjects($.get(state) /**/),
		onStateChange: (updater) => {
			$.set(state, $.proxy(mergeObjects($.get(state), updater), null, state));
		}
	});
}

So first thing we do is set state to contain a proxied version of the result getInitialTableState.
As a result, state is not really a normal thing anymore, it's basically a pointer to a proxy. Let's call this version of the proxy '1', and we'll say that state is pointing to the version 1 proxy.

Now, we set up statefulOptions.
$.get is going to return the $.proxy that just got created. More specifically, it's going to return a reference to proxy version 1. Not to state itself.

We then spread out all the results of the proxy, which copies the contents of the proxy. Your mergeObjects suggestion doesn't do that, which is good! The way rewrites/etc happen in svelte 4 (which is quite different) would make it work even!

But it will still not work in svelte 5. Obviously the copied proxy pieces will not update. But let's imagine we didn't do a spread, and just pass state to mergeObjects. Or even simpler, we just use it directly here.
if this was state: $.get(state), we would have this state property being a property that contains a reference to proxy version 1.

Note that once statefulOptions is created, it's done. Nothing else in this function is going to make this state property point to anything other than proxy version 1, with or without a spread.

Next up, updateOptions. You can actually see what mergeObjects would do on its own here. In this case, $.get(state) is referencing the state from above (it's a non-obvious closure). Because updateOptions is a function, every time it's called, it is re-running this code, and re-getting the current version of the proxy from state. So this will update as state updates, at least, kind of.
It will update on every function call, but it will point to an old version anytime onStateChange is called, until updateObject is called again.
So on the first call, the state property here will point to version 1.

But you can then see what happens. The $.set creates a new $.proxy, and sets state to this proxy, effectively incrementing the proxy version of state. So now the original state variable state is pointing to proxy version 2. The property we just set is pointing to proxy version 1.
Our state (spread or not) back in statefulOptions is also still a reference to proxy version 1.

Let's call updateObject again.
Now our state property here will call $.get again, and be pointing to proxy version 2.
statefulOptions is still a reference to proxy version 1.

At this point, the state property in updateObject will be up to date until onStateChange is called.
Once you call onStateChange, it will fall behind again until updateObject is called.
As you can see, the state property in statefulOptions will never update. That is the literal meaning of the warning, as confusing as it is to people who have not jumped down tremendous rabbit holes to understand what it is trying to say.
(I worked on C/C++/Java/etc compilers for over a decade, so this sort of weirdness is sort of second nature to me. I will say i think we got very far to improving, say, C++ error messages vs 20 years ago. This kind of message feels like the C++ errors of old that were not that useful)

Now you can also see (hopefully) why my fix is still buggy, despite making the warning go away :). While it will lazily evlauate, it will not do it at the right time.
Whoops.

We have to use a property getter instead.

Then the code will be

const statefulOptions = mergeObjects(tableOptions, {
	get state(): { return  ...$.get(state) }
});

As you can see, now every time the property is accessed on statefulOptions, that is when we will go and get the current proxy version from state, and spread it. That will always return the up-to-date proxy version, and then spread that proxy version.

A similar thing needs to be done for the state property in updateObjects.

I'll update the fix.

Assuming, again, there isn't a reason none of this matters :)

[walker-tx]

@dberlin I love the thorough write-up. What you're describing makes sense - learning how runes/signals work was a big challenge for me in setting this adapter up, so it's not a surprise that it would continue to be. :)

Would you go ahead and apply the updates that you mentioned above?