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 classes to opt-in to deep reactivity #11846

Open
jrpat opened this issue May 30, 2024 · 12 comments
Open

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

jrpat opened this issue May 30, 2024 · 12 comments

Comments

@jrpat
Copy link

jrpat commented May 30, 2024

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:

  • Use grid[coord.row]?.[coord.col] throughout the app
  • Turn the helper methods into free/static functions like Grid.at(grid, coord)
  • Do something like
    let g = $state(foo.grid.data)  // grid.data is the 2d array
    foo.grid.data = g
    // repeat for all non-POJO objects in nested state
    // then finally
    let reactiveFoo = $state(foo)

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
Copy link
Member

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

@jrpat
Copy link
Author

jrpat commented May 31, 2024

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

@brunnerh
Copy link
Member

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
Copy link
Author

jrpat commented May 31, 2024

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
Copy link

jycouet commented Oct 17, 2024

(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
Copy link
Member

@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
Copy link

jnhrrj commented Nov 24, 2024

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

@cristianvogel
Copy link

cristianvogel commented Dec 2, 2024

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.)

Hi,

I am trying to migrate a UI project that also uses Classes and inheritance. I am running into this issue.

When I try what you suggest in my Class field, at runtime I get an error
$state(...) can only be used as a variable declaration initializer or a class field

from the source code

export class BasicController extends PrecisController {
    currentValue = $state<number>(0);
    taper: Taper
    id = $state<string | undefined>();

    constructor() {
        super()
    }

which seems to get re-written to

  File: src/lib/PrecisControllers.svelte.ts:197:19
   113 |    constructor() {
   114 |      super();
   115 |      this.currentValue = $state(0);
                                           ^
   116 |      this.id = $state();
   117 |    }

I don't really understand why this doesn't work for me?

.. later

OK, I thought about it, and tried this way. To make a function in the base class that returns the reactive state unwrapped, as described in the docs.

    private getValue( widget: BasicController ): { mapped: number, normalised: number } {
        let current = $state(widget.currentValue);
        return {
            get mapped(): number {
                return remap(current / widget.height, 0, 1, widget.taper.min, widget.taper.max)
            },
            get normalised(): number {
                return (current / widget.height)
            }
        }
    }

    getMappedValue(): number {
        return this.getValue(this).mapped
    }

    getNormValue(): number {
        return this.getValue(this).normalised
    }

But it still doesn't respond... stuck.

@brunnerh
Copy link
Member

brunnerh commented Dec 2, 2024

You need a TS config that does not mess with the class definition.
See this answer (docs issue: #14187).

@cristianvogel
Copy link

Thanks, that totally got things unstuck!

@dummdidumm
Copy link
Member

Related #10560

@gian-didom
Copy link

I have just been introduced to Svelte5 and I find the lack of reactivity for instances of class something to be addressed for sure.

In other frameworks (and in JS in general), classes have just been Objects on steroids, keeping all the compatibility with most operations you could perform on Objects. This is fairly common in other languages as well, such as C++ in which classes are just structures with public and private access.

This means that lots of plain JS libraries chose to use classes instead of {} and are now just kept outside the Svelte environment since one would need to re-write custom interfaces/wrappers for each one of them, or give up reactivity at all.

I think making some kind of helper, as proposed above, would be a convenient solution to keep compatibility with existing class-based JS libraries.

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

7 participants