sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
80.28k stars 4.27k forks source link

Svelte 5: Allow classes to opt-in to deep reactivity #11846

Open jrpat opened 6 months ago

jrpat commented 6 months ago

Describe the problem

I understand the rationale for $state not proxying non-POJO objects, but I think having non-POJO reactive state is a very useful pattern that is worth trying to support.

A very simple example (and my current use case): Part of state is a Grid which wraps a 2d array and provides helper methods like grid.at(coord: Coord). By forcing state to be only POJO, I now have to do one of:

There are loads of example use-cases like this, as it's a pretty common data modeling pattern (usually a Model pattern is just "some raw data + some helper methods to access/modify the raw data").

Describe the proposed solution

For the sake of nomenclature, let's just refer to classes and class instances, though of course classes aren't the only way to create objects with a non-Object prototype.

It would be nice to allow classes to opt in to being "proxy-able", either by specifying that the proxy should wrap the entire object, or by specifying some subset of fields to be proxied.

A potential syntax: (exact naming TBD)

class Thing {
  foo: number;
  bar: string;
}

// Assume `$state.proxyable` is a Symbol.

// Option 1: Proxy the object, and let the user handle errors 
// which originate from private/internal property accesses:
Thing.prototype[$state.proxyable] = true

// Option 2: Proxy some subset of fields from the object:
Thing.prototype[$state.proxyable] = ['foo', 'bar']

Then, when proxy checks whether to wrap the object, it can also check prototype[$state.proxyable] and proceed accordingly.

Option 1 downsides: Users may hit errors resulting from private/internal property accesses, arguably making $state a slightly leakier abstraction than it currently is.

Option 2 downsides: The list of proxied fields would also have to be stored in the metadata and queried during various proxy traps, which is less than ideal from a performance perspective.

Importance

nice to have

dummdidumm commented 6 months ago

Is Grid under your control? Or is this for classes not under your control?

jrpat commented 6 months ago

It is under my control. Is there a better alternative I'm missing?

brunnerh commented 6 months ago

I guess the question then is, whether there is a reason not to make the fields of the class stateful?

Given the example thing:

class Thing {
-  foo: number;
+  foo = $state<number>();
-  bar: string;
+  bar = $state<string>();
}

(Would need assertions if you want to prevent undefined in the type.)

jrpat commented 6 months ago

The reason not to go that route is that Grid (and Coord and many others) is a utility class that is used in many places, only some of which are reactive state.

One might say this is a userland problem and could be solved pretty robustly with a simple helper. Something like:

let reactiveObj = $state(deeplyMakeStateful(plainObj))

where deeplyMakeStateful walks the object tree and wraps appropriate fields in $state (using some mechanism like the proposal above).

And that does work, but now we're walking the object tree twice (worse performance) and adding boilerplate (unintuitive) wherever $state is involved. It seems like a win if we can provide a simple and lightweight built-in way to do this.

jycouet commented 1 month ago

(Would need assertions if you want to prevent undefined in the type.)

How is it possible with $state(), where do you do the check ?


I'm using some libs (not Svelte specific) that works a lot with classes. I started with this approach:

let myObj = $state<MyObj | undefined>()

const refresh = async (_slug: string) => {
  const p = await repo(Package).findFirst({ slug: _slug }, { include: { items: true } })
  if (p) {
    // TODO: make it generic...
    myObj = {
      ...p,
      items: (p.items?? [])?.map((i) => {
        return { ...i }
      }),
    }
  } else {
    goto('/', { replaceState: true })
    throw "Doesn't exist!"
  }
}

$effect(() => {
  refresh($page.params.slug)
})

Would you share your deeplyMakeStateful ? Or is there a better way today?

Thx for you help.

brunnerh commented 1 month ago

@jycouet You can e.g. use a definite assignment assertion:

class Counter {
  value: number = $state()!; // note `!` at the end
  constructor(initial: number) {
    this.value = initial;
  }
}
jnhrrj commented 1 week ago

I am still unsure how to handle reactivity for instances of classes that are not under my control. I have the feeling and hope that there is a better way not mentioned in the docs