Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back $state.is rune #13582

Closed
kwangure opened this issue Oct 12, 2024 · 11 comments
Closed

Bring back $state.is rune #13582

kwangure opened this issue Oct 12, 2024 · 11 comments

Comments

@kwangure
Copy link
Contributor

kwangure commented Oct 12, 2024

Describe the problem

The $state.is() rune was removed with the argument that $state.raw makes it not needed. In #12916 (comment) Rich says:

Write your code in such a way that you don't need to compare a proxy with a non-proxy...
....
$state.raw is a much better approach:

<script>
  let items = [...];
- let selected = $state(items[0]);
+ let selected = $state.raw(items[0]);
</script>
{#each items as item}
<div class:selected={selected === item}>...</div>
{/each}
  1. This "fix" is fine in a single component but not in a sizable application where data is crossing several boundaries where anything could proxify it. (e.g item[0] being passed to ListItem and then createListItem() where I want to do const active = $derived(myRootComponent.value === item) and return { get foo() { return item.deep.value.foo; }
  2. 90% of state starts as $state only for you to later realize you need to compare it with something downstream.
  3. In the context of Rich's example, the particular use case I'm interested in is where you want items[0] to be a reactive signal/proxy...but also compare it with selected.

Expecting someone to just "Write your code in such a way that you don't need to compare a proxy with a non-proxy" is not a reasonable request because:
a) It needs engineering time to refactor the entire state & component tree that touches a piece of data potentially proxying it while $state.is would otherwise suffice.
b) There's no easy way to enforce that distant up-stream or down-stream consumers not to proxy data.
c) I now need to document and recall arguments/props that should not be deep proxies as I'm passing them to places that do comparison...and I need to actually know if they do comparison
d) Probably most importantly, my data is already proxied, I want it to be deeply reactive, but I also want to compare it. What can we do about it?

Describe the proposed solution

Bring $state.is back. Alternatively, some other solution that allows dealing with proxied data after the fact and preserves object identity (e.g $state.unproxify) is also welcome.

Importance

i cannot use svelte without it

@trueadm
Copy link
Contributor

trueadm commented Oct 14, 2024

$state.is won't do anything anymore, as we no longer mutate the original object passed into $state.

@brunnerh
Copy link
Member

Svelte would probably need to track object-proxy associations in an internal WeakMap (using WeakRef values).

@kwangure
Copy link
Contributor Author

If it's not too much to ask, kindly ELI5 the whys and hows of "mutating the original object" affects this issue or separate tracking is needed (for my edification).

I don't have enough context to know why might simply exporting is is not enough.

export function is(a, b) {

@trueadm
Copy link
Contributor

trueadm commented Oct 14, 2024

So when you pass the object to $state() we essentially make a proxy clone of that object. So it's an entirely new object from the original, and all objects inside that object (deeply) are new proxies clones too if they're simple objects/arrays. So they no longer have equality === and we can't compare them anymore, as they're now entirely new things. Making mutations to the original object will not be reflected in the proxied objects and vice versa.

@kwangure
Copy link
Contributor Author

kwangure commented Oct 14, 2024

Convince me more:

Svelte REPL

@trueadm
Copy link
Contributor

trueadm commented Oct 14, 2024

Convince me more:

If you read the field then it becomes a new object. Proxying deep is lazily done, not eagerly, so if you read it then it will change identity and thus goes back to my above points.

@kwangure
Copy link
Contributor Author

Okay. Convinced and edified.

@kwangure
Copy link
Contributor Author

I discovered the STATE_SYMBOL trick in the REPL above today in frustrated experimentation. Even knowing about this issue, I still sunk more time into it because bugs comparing proxies require client-side interactions to be noticed. Comparison deceptively succeeds in SSR because signals are stripped away on the server so things seem to be working fine on first load.

@trueadm
Copy link
Contributor

trueadm commented Oct 14, 2024

This is about like each blocks – in where we needs keys to understand identity. If you can make your objects use an id or some form of key then you can easily compare identity in both places. Unfortunately, proxies can be a misleading, which is why we changed our approach from before to ensure that we didn't mutate the underlying object when you mutate the proxied object – as this was causing very unfortunate side-effects that were almost impossible to debug.

I don't think there's a silver bullet or an additional API here that can make this problem go away. If you have any ideas, please let us know, but $state.is is most definitely not the right thing here.

@kwangure
Copy link
Contributor Author

kwangure commented Oct 14, 2024

Hmm. I'm writing a library where I receive arrays with unknown/arbitrary elements passed down a hierarchy much like the ListItem in the initial post.

I think rather than accept unknown[] and comparing the values for identity the compromise for now will be to require something like { id: string, value: unknown }[] and compare the user provided id/keys instead.

@trueadm
Copy link
Contributor

trueadm commented Oct 17, 2024

That's a good approach. Closing this now as nothing is actionable :)

@trueadm trueadm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants