Skip to content

Assume every variable might be dynamic and therefor render all of them inside a render_effect(). #11016

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
Evertt opened this issue Apr 1, 2024 · 4 comments
Milestone

Comments

@Evertt
Copy link

Evertt commented Apr 1, 2024

Describe the problem

This is actually a smaller feature request that I'm asking for, in case #11014 for some reason won't get accepted. Please first check out #11014, because I care more about that one and if that one gets accepted then this one will be moot.

My frustrations are actually about how Svelte 5's current design makes it impossible for us (the community) to expand on Svelte's primitives (i.e. its runes / underlying signals), creating more primitives that offer the same ergonomics as Svelte 5's native primitives, like being able to do this:

let count = $state(0)

function increment() {
    count = count + 1
}

Instead of:

<script lang="ts">
const count = ref(0)

function increment() {
    count.value = count.value + 1

    // or as some people prefer
    // count(count() + 1)
}
</script>

<button onclick={increment}>
    The count is at {count.value}
</button>

But unfortunately, for us who want to create entire libraries full of novel reactive utilities, similar to VueUse and Solid Primitives... We have no choice but to use one of those less ergonomic syntaxes.

Fortunately though, there is a solution that could solve almost all the ergonomic problems. Namely, if our ref() could return an object that includes methods such as toString() and valueOf() and [Symbol.toPrimitive](hint), then there are many cases in which we could treat count as if it were simply the primitive value that it is wrapping.

It doesn't fully fix the mutating scenario of a ref wrapping a primitive value. Which is why, again, I hope that #11014 will get accepted. But in any case it should fix almost all of the reading scenarios. For example, one could rewrite increment to this:

function increment() {
    count.value = count + 1
    // or count(count + 1)
}

And one should be able to simply use {count} anywhere in the template and it should simply print out the primitive value wrapped by count, thanks to those methods we added to the object.

However, that does not always work! Because currently Svelte tries to make educated guesses about which values might be reactive and sometimes it will guess (incorrectly!) that a value is probably not reactive and then it will render that value in the template outside of a render_effect(), which will make it not reactive.

Describe the proposed solution

Simple, just assume any variable might be reactive, because you simply can't know for sure.

In the worst case scenario, if the variable is indeed not reactive, you lose a tiny bit of performance on the first render (because of the overhead of render_effect()). But after that it won't have any effect on anything whatsoever, because the non-reactive value will never change and so the render_effect() will never re-run.

But that will give us, the community, the opportunity to create ref() functions that return reactive things that use all the tricks we can think of to make the ergonomics of using those reactive things the best we can make them.

Importance

nice to have

@7nik
Copy link
Contributor

7nik commented Apr 1, 2024

Though I have opened issue about using this approach, I vote against it. I have tried the same trick to make a work with stores in both .svelte and .js/.ts files simple and unified. It worked, but I had to keep an eye on what and how I write, which isn't good, especially for lib consumers.

Firstly, all these fake primitives will have type object and "fail" check on typeof.
Secondly, equal checks became tricky:

let x1 = 42;
let x2 = wrap(42); // add custom toString, valueOf, @@toPrimitive but now it's an object
let x3 = wrap(42);
console.log(1, x1 == x2);
console.log(2, x1 === x2);
console.log(3, x1 == +x2);
console.log(4, x2 == x3);
console.log(5, x2 == +x3);
console.log(6, x2 === +x3);
console.log(7, +x2 == +x3);
console.log(8, +x2 == x3 + "");

Now, guess the outputs.

Answers:
1 true
2 false
3 true
4 false
5 true
6 false
7 true
8 true

Thirdly, boolean became the most insidious type because it's used in conditions (x ? 1 : 2, if (x) ..., {#if x} ...., etc) but it always is true. Similarly, wrappedString || "default value" will never fallback to the default value. It's easy to miss such a case and then spend hours looking for a bug.

@Rich-Harris Rich-Harris added this to the 5.0 milestone Apr 1, 2024
@Evertt
Copy link
Author

Evertt commented Apr 2, 2024

@7nik Those are all very fair points, I can't really deny that. I'm not yet sure if it 100% negates the usefulness of my request, so I'd like to hear other people's thoughts too. But I do see that indeed returning an object from let x = wrap(42) with toString(), valueOf(), and @@toPrimitive() might lull the user into a false sense of security when they are using the variable x in the most convenient ways possible.

@Rich-Harris
Copy link
Member

We've experimented with valueOf and friends and reached the same conclusion — it's confusing and dangerous and we don't want to encourage its use.

For example, one could rewrite increment to this:

function increment() {
    count.value = count + 1;
}

Please don't write code like that! It's unnecessarily confusing, and TypeScript will think you're a lunatic.

Aside from the design reasons not to do this, it would introduce unnecessary overhead, making everyone's apps larger and slower than they need to be. The fewer effects we need to create, the more memory-efficient we can be.

@dummdidumm
Copy link
Member

We decided we don't want to do this, therefore closing

@dummdidumm dummdidumm closed this as not planned Won't fix, can't repro, duplicate, stale May 3, 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

4 participants