affectedToPathList doesn't work with classes?

[stephenh]

This is potentially a proxy-compare issue, but starting the discussion here b/c that repo doesn't have discussions enabled.

I'm hoping to use classes as basically a more succinct proxyWithComputed, i.e. with methods/getter/setters working just like the 2nd arg of computeds (...basically exactly like your GameState demo).

However, while just console.log-ing the internals of valtio to understand how it works, I was not seeing any "getters" show up in affectedToPathList.

For example, isolating a repro just inside of proxy-compare, this example with an object literal shows accesses are recorded to fullName:

  it('pathList with literal', () => {
    const proxyCache = new WeakMap();
    const s1 = {
      firstName: 'f',

      get fullName() { return this.firstName.toUpperCase(); },
    };
    const a1 = new WeakMap();
    const p1 = createProxy(s1, a1, proxyCache);
    noop(p1.fullName);
    expect(affectedToPathList(s1, a1)).toEqual([['fullName']]);
  });

The fullName access shows up; however, if I use a class, it does not:

  it('pathList with class', () => {
    const proxyCache = new WeakMap();
    class Author {
      firstName = 'f';

      get fullName() { return this.firstName.toUpperCase(); }
    }
    const s1 = new Author();
    const a1 = new WeakMap();
    const p1 = createProxy(s1, a1, proxyCache);
    noop(p1.fullName);
    noop(p1.firstName);
    expect(affectedToPathList(s1, a1)).toEqual([]);
  });

Is this expected?

Fwiw I expected this to break using classes in useSnapshot; i.e. because I don't see fullName show up in affectedToPathList, I thought mutations/new snapshots that changed only fullName would not re-render ... but they do!

Maybe I should just trust the magic :-), but, dunno, I want to use heavily use classes with valtio, so just wanted to make sure there were not any gotchas.

Thank you!

[dai-shi]

This is potentially a proxy-compare issue, but starting the discussion here b/c that repo doesn't have discussions enabled.

Yes, it's fine.

I'm hoping to use classes as basically a more succinct proxyWithComputed, i.e. with methods/getter/setters working just like the 2nd arg of computeds (...basically exactly like your GameState demo).

No, proxyWithComputed and classes are very different.

But, in the following example, you compare object getters and class getters. They are mostly the same.

However, while just console.log-ing the internals of valtio to understand how it works, I was not seeing any "getters" show up in affectedToPathList.

Yes, class getters are in prototype, so it's not accessed. So, it's expected.

For example, isolating a repro just inside of proxy-compare, this example with an object literal shows accesses are recorded to fullName:

  it('pathList with literal', () => {
    const proxyCache = new WeakMap();
    const s1 = {
      firstName: 'f',

      get fullName() { return this.firstName.toUpperCase(); },
    };
    const a1 = new WeakMap();
    const p1 = createProxy(s1, a1, proxyCache);
    noop(p1.fullName);
    expect(affectedToPathList(s1, a1)).toEqual([['fullName']]);
  });

The fullName access shows up; however, if I use a class, it does not:

  it('pathList with class', () => {
    const proxyCache = new WeakMap();
    class Author {
      firstName = 'f';

      get fullName() { return this.firstName.toUpperCase(); }
    }
    const s1 = new Author();
    const a1 = new WeakMap();
    const p1 = createProxy(s1, a1, proxyCache);
    noop(p1.fullName);
    noop(p1.firstName);
    expect(affectedToPathList(s1, a1)).toEqual([]);
  });

Is this expected?

Isn't the last expect like expect(affectedToPathList(s1, a1)).toEqual(['firstName']);

Hm, is it because not calling snapshot() before createProxy? That's how useSnapshot does.

- const p1 = createProxy(s1, a1, proxyCache);
+ const ss1 = snapshot(s1);
+ const p1 = createProxy(ss1, a1, proxyCache);

Fwiw I expected this to break using classes in useSnapshot; i.e. because I don't see fullName show up in affectedToPathList, I thought mutations/new snapshots that changed only fullName would not re-render ... but they do!

Maybe I should just trust the magic :-), but, dunno, I want to use heavily use classes with valtio, so just wanted to make sure there were not any gotchas.

You can use React Dev Tools to see "affected" in useSnapshot. If it has firstName (not fullName), then it's working as expected.

[stephenh]

Thank you for the quick reply!

No, proxyWithComputed and classes are very different.

Ah okay. I want to respectful of your time, so can you point to any docs/threads that explain the difference and pros/cons of each? Or also happy to get a tldr.

Naively, just looking at the GameStep demo, it seemed like there were "class getters doing what computeds do", and "class setters mutating what actions would do".

Isn't the last expect like expect(affectedToPathList(s1, a1)).toEqual(['firstName']);
...
Hm, is it because not calling snapshot() before createProxy?

Hm, no it really is [], and I'm running this test directly in proxy-compare, the 15_nested.ts file, so valtio's snapshot isn't around.

Ah okay, I had to markToTrack it; which is something that snapshot does to:

  it('pathList with class', () => {
    const proxyCache = new WeakMap();
    class Author {
      firstName = 'f';

      get fullName() {
        return this.firstName.toUpperCase();
      }
    }
    const s1 = new Author();
    markToTrack(s1);
    const a1 = new WeakMap();
    const p1 = createProxy(s1, a1, proxyCache);
    noop(p1.fullName);
    expect(affectedToPathList(s1, a1)).toEqual([["fullName"]]);
  });

So, good, now it does record fullName (although not firstName, which I kinda thought would get recorded b/c the this in this.firstName would be the proxy).

But, back in valtio, it still seems like valtio using createProxyToCompare against a snapshot (instead of the raw class instance like I'd done in my test) means the getter calls are not tracked.

Here is a ~long-ish example, and it's just a unit test, but if I put console.logs into useAffectedDebugValue, the getters, nor the fields the getter access, ever show up in pathList (...but the reactivity still "just works" and doesn't re-render if bigTitle is commented out in the JSX):

import { act, fireEvent, render } from "@testing-library/react";
import React from "react";
import { proxy, useSnapshot } from "valtio";

class Book {
  title = "b1";
  pages = 10;
  get bigTitle() {
    return this.title.toUpperCase();
  }
}

class Author {
  age = 40;
  books = [new Book()];
  get oldAge() {
    return this.age * 2;
  }
}

const state = proxy(new Author());

function TestComponent() {
  const snap = useSnapshot(state);
  function incrementAge() {
    console.log("incrementAge");
    state.age++;
  }
  function changeTitle() {
    console.log("changeTitle");
    state.books[0].title += ".";
  }
  return (
    <div>
      <div data-testid="age">{snap.age} {snap.oldAge}</div>
      <div data-testid="bigTitle">{snap.books[0].bigTitle}</div>
      <div data-testid="incrementAge" onClick={incrementAge} />
      <div data-testid="changeTitle" onClick={changeTitle} />
    </div>
  );
}

describe("ValtioApp", () => {
  it("doesn't render if computed doesn't change", async () => {
    const r = await render(<TestComponent />);
    await clickAndWait(r.getByTestId("incrementAge"));
    await clickAndWait(r.getByTestId("changeTitle"));
  });
});

async function clickAndWait(e: Element): Promise<void> {
  await act(async () => {
    fireEvent.click(e);
    await new Promise(jest.requireActual("timers").setImmediate);
  });
}

[dai-shi]

Ah okay, I had to markToTrack it; which is something that snapshot does to

Good catch!

[dai-shi]

#642 (comment)

Ah okay. I want to respectful of your time, so can you point to any docs/threads that explain the difference and pros/cons of each? Or also happy to get a tldr.

proxyWithComputed and derive create a real proxy.

So from valtio point of view, it's identical to proxy({ firstName: 'f', fullName: 'F' }).

Object getters and class getters are just functions, so it's identical to the following:

const state = proxy({ firstName: 'f' });
const Component = () => {
  const snap = useSnapshot(state);
  const fullName = snap.firstName.toUpperCase();
  // ...
};

[stephenh]

proxyWithComputed and derive create a real proxy.

Hm, okay, thanks! I get that now...

I guess that seems like a somewhat minor implementation detail/approach difference? Like I'm not seeing anything that, for me, would scare me away from using classes, or consider either proxyWithComputed or classes that sufficiently different/more/less powerful than the other.

Other than proxyWithComputeds ability to provide additional memoification of the return value itself.

I tried to write up a AFAIU/tldr of the "proxyWithComputed vs. classes" in my latest docs PR; happy to have you correct my understanding either here or in the PR.

...fwiw I am honestly a little surprised that snapshot doesn't auto-memoize getters. This is what mobx did, at least by default, so it's what I expected coming in. Totally get your point that perf should be measured instead of guessed, and I'm sure ~80%-95% of getters are cheaper to just re-calc vs. the overhead of memozing, but 🤷, just talking out loud.

Thanks!

[dai-shi]

I guess that seems like a somewhat minor implementation detail/approach difference?

Yes, using class getters on snapshots are valid/supported usage. (It's just this is confusing, and not recommended for JS beginners.)

I'm sure ~80%-95% of getters are cheaper to just re-calc vs. the overhead of memozing

I think so too. In your use case, proxyWithComputed / derive are not necessary. Sorry, if something confused.

[stephenh]

In your use case, proxyWithComputed / derive are not necessary.

Great!

Sorry, if something confused.

Np! As a newbie I just couldn't immediately intuit the pros/cons of classes vs. proxyWithComputed from just reading the docs, which hopefully my docs PR tries to clarify a bit. Happy to make any corrections/edits!

[dai-shi]

I'll have a look on the PR later.

[stephenh]

Going to close this discussion out b/c I found what was tripping me up:

#545 (comment)

It'd be interesting to poke at fixing someday, but for now it just tripped up my learning curve.

I also included an FYI about the behavior in my 2nd docs PR.

Thanks!