Skip to content

Support Composability on Runes #12917

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

Closed
mrh1997 opened this issue Aug 20, 2024 · 13 comments
Closed

Support Composability on Runes #12917

mrh1997 opened this issue Aug 20, 2024 · 13 comments

Comments

@mrh1997
Copy link

mrh1997 commented Aug 20, 2024

Describe the problem

In #9651 there was already a discussion on building "Higher Order Stores with Runes" which resulted in the recommendation of passing functions that return runes. But as already mentioned in this recommendation this solution is only unidirectional (no binding possible).

You could implement this bidirectional via creating an wrapper object with a getter and a setter. But passing a parameter to this composable ("double") is very ugly:

const comp = double({get value() { return count; }, set value(v) { count = v; }});

Especially when working with nested composables (a(b(c))) this approach gets totally unreadable.

Describe the proposed solution

It would be great if there would be a boxing rune that does the wrapping for me:

const comp = double($wrap(count));

Furthermore I would like to see a recommendation/best practice from sveltejs which name to use for the attribute so that all composables will use the same resolving mechanism. In my proposal this is .value but fromStore() uses .current. This should be standardized to make composables composable...

Importance

would make my life easier

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

We've been through this many times before and we've even implemented a similar rune and played around with it – it just doesn't work too well. The issue here is you're trying to box a primitive value, but in reality, this is mostly down to the pattern of creating primitives as singular bits of state that need to be passed around.

Typically, you'll be dealing with many state fields that are usually interlinked, so it might make more sense to put these into $state or a class instead. i.e. let person = $state({ name, age, location }). So the benefits of having a dedicate rune for just primitives becomes less important in reality. Not to mention $wrap has so many gotchas around debugging – it's hard to put in a debugger or console.log to understand when and how it changes and the call-stack you get from errors becomes removed from the source-code – unlike functions which work great.

@trueadm trueadm closed this as completed Aug 20, 2024
@trueadm trueadm reopened this Aug 20, 2024
@trueadm trueadm closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2024
@david-plugge
Copy link

Its actually quite easy to create a vite plugin that does exactly this. The downside ist that svelte tells you about state beeing used outside an effect.
It would be pretty cool to have more control over how state is handled. Imagine a component library that exposes classes to encapsulate state. How do you sync the outside and inside state? Using a Box! But manually wrapping all Input states in boxes gets kinda annoying.

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

Its actually quite easy to create a vite plugin that does exactly this. The downside ist that svelte tells you about state beeing used outside an effect. It would be pretty cool to have more control over how state is handled. Imagine a component library that exposes classes to encapsulate state. How do you sync the outside and inside state? Using a Box! But manually wrapping all Input states in boxes gets kinda annoying.

We actually don’t recommend using a box. Having a box for each state variable is highly ineffective and having to do .value all over your codebase is just not great. Really there are better ways.

@david-plugge
Copy link

david-plugge commented Aug 20, 2024

Interesting. How else should i approach my described use case then?

@mrh1997
Copy link
Author

mrh1997 commented Aug 20, 2024

Typically, you'll be dealing with many state fields that are usually interlinked, so it might make more sense to put these into $state or a class instead. i.e. let person = $state({ name, age, location }). So the benefits of having a dedicate rune for just primitives becomes less important in reality.

Composability unfolds its power when the composables are working on primitives and are doing exactly one job (i.e. sorting, filtering, ...). This makes them highly reusable (see haskells standard library).

It does not make sense to write composables for classes since they normally only makes sense in the context of that class and thus better are implemented as methods for that class.

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

Typically, you'll be dealing with many state fields that are usually interlinked, so it might make more sense to put these into $state or a class instead. i.e. let person = $state({ name, age, location }). So the benefits of having a dedicate rune for just primitives becomes less important in reality.

Composability unfolds its power when the composables are working on primitives and are doing exactly one job (i.e. sorting, filtering, ...). This makes them highly reusable (see haskells standard library).

It does not make sense to write composables for classes since they normally only makes sense in the context of that class and thus better are implemented as methods for that class.

I get what you're saying but this is just a fundamental flaw of JavaScript, in that you can't pass primitives by reference. There have been proposals at TC39 to add such support but there's always been resistance in the past because you can just use functions.

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

Interesting. How else should i approach my described use case then?

If you can give a more concrete real-world example, that would be perfect.

@david-plugge
Copy link

This is a very basic example of what i'm talking about. Im pretty sure shadcn-svelte also does it in a similar way in their svelte 5 version.

<script lang="ts" context="module">
	import type { FloatingConfig } from '$lib/behaviours/floating';
	import { readableBox, writableBox, type ReadableBox, type WritableBox } from '$lib/box';

	interface SelectOptions {
		open: WritableBox<boolean>; // writable box -> we want to write to this value from inside AND outside
		floatingConfig: ReadableBox<FloatingConfig>;  // readonly, we dont need to write from inside our class
	}

	export class Select {
		#open: WritableBox<boolean>;
		#floatingConfig: ReadableBox<FloatingConfig>;

		constructor(options: SelectOptions) {
			this.#open = options.open;
			this.#floatingConfig = options.floatingConfig;
		}

		get open() {
			return this.#open.value;
		}
		set open(v) {
			this.#open.value = v;
		}

		trigger() {
			return {
				onclick: () => {
					this.#open.value = !this.#open.value;
				}
			} as const;
		}

		listbox() {
			return {
				// use floating config, ...
			} as const;
		}
	}
</script>

<script lang="ts">
	interface Props extends FloatingConfig {
		open?: boolean;
	}
	let {
		open = $bindable(false),

		// floatingConfig
		placement,
		flip
	}: Props = $props();

	const floatingConfig = $derived({
		placement,
		flip
	});

	const select = new Select({
		floatingConfig: readableBox(floatingConfig),
		open: writableBox(open)
	});
</script>

<button {...select.trigger()}>...</button>
<div {...select.listbox()}>...</div>

@david-plugge
Copy link

I get what you're saying but this is just a fundamental flaw of JavaScript, in that you can't pass primitives by reference. There have been proposals at TC39 to add such support but there's always been resistance in the past because you can just use functions.

So functions are more performant than boxes using getters and setters?

I do understand that js is the limitation here, but i do have interest in a svelty solution or atleast a recommended way to solve my use case.

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

So functions are more performant than boxes using getters and setters?

Depends on if the getters/setters are on the prototype or not. If they're not, they're not fast at all.

svelty solution

The team have dug into this numerous times. If we had found a Svelty solution, you'd have it already. This was the closest we came #11210.

@david-plugge
Copy link

So to sum it up we do not have an easy way of solving this issue at the moment. The most performant way should be letting the end user of my example library above create a state object (or even better a class instance to use less proxies) that gets passed into the component. Basically creating the state at the first point of usage in the component tree.
Is that correct?

@trueadm
Copy link
Contributor

trueadm commented Aug 20, 2024

Yeah, I think we're talking about the same thing. I think there's no silver bullet for this too and organically newer patterns will merge over time and we'll maybe revisit this topic when that happens.

@david-plugge
Copy link

Alright, thank you very much for your quick responses👍

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