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

Svelte 5: Allow returning (object) $state directly? #12286

Closed
brunnerh opened this issue Jul 3, 2024 · 2 comments
Closed

Svelte 5: Allow returning (object) $state directly? #12286

brunnerh opened this issue Jul 3, 2024 · 2 comments

Comments

@brunnerh
Copy link
Member

brunnerh commented Jul 3, 2024

Describe the problem

I have often run into a pattern where utility functions creating a stateful object create the object and immediately return it.
This is currently required as $state is only allowed in variable/ class field declarations.

E.g.

export function createSelection() {
	const selection = $state({
		items: [],
		isSelected(item) {
			return this.items.includes(item);
		},
		toggle(item) {
			this.items = this.items.includes(item)
				? this.items.filter(x => x != item)
				: [...this.items, item];
		},
	});

	return selection;
}

Describe the proposed solution

Allow return $state(), but maybe only if used with an object literal so problems like usage with primitives are caught more easily.

Importance

nice to have

@Rich-Harris
Copy link
Member

We've spent a lot of time considering this as you can imagine, and have decided against it thus far. If you restrict it to object/array literals, then valid refactorings become invalid for reasons that don't make sense unless you're deeply versed:

// can't turn this...
return $state({...});

// ...into this, say if you temporarily need to debug the value before returning it:
let blah = {...};
debugger;
return $state(blah);

You could make the static analysis smarter, but that becomes a game of whac-a-mole. Or you could do it at runtime, though even if you only do that in the ambiguous cases it adds overhead (and if you only do it in dev, you've introduced a new difference between dev and prod).

So it's not totally obvious that it's worth it, even though I've found myself wishing for it in the past.

I would argue that a better pattern in most cases is to wrap createSelection() rather than the return value:

export function createSelection() {
-  const selection = $state({...});
-  return selection;
+  return {...};
}

-const selection = createSelection();
+const selection = $state(createSelection());

@trueadm
Copy link
Contributor

trueadm commented Jul 4, 2024

I actually implemented this a while back and it wasn't obvious at all statically. Especially if you reference an initial value from a function argument. It caused all sorts of confusion, and that was why we closed the PR. So on that basis, I don't think we need to revisit this as the outcome will be the same as last time.

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