fix: Proxy model not setting defined values in parent class (#2187)

This is only an issue if we directly set a public member. In the current
proxy model, we only set via `setField` type functions, so things seem
to be fine. Inside of class definitions the proxy is skipped (since the
proxy is just returned from the ctor).

This change makes setting a member defined in the proxy class work even
if you try to do it on the proxy object.

```ts
class ProxyModel {
  myField = 4
}

test = new ProxyModel();
test.myField; // 4
test.model.myField; // undefined

test.myField = 5;
test.myField; // 4 - WRONG
test.model.myField; // 5
```

Author: mattrunyon

packages/iris-grid/src/IrisGridProxyModel.test.ts

@@ -0,0 +1,167 @@
+import dh from '@deephaven/jsapi-shim';
+import IrisGridModel from './IrisGridModel';
+import IrisGridProxyModel from './IrisGridProxyModel';
+import IrisGridTestUtils from './IrisGridTestUtils';
+
+const irisGridTestUtils = new IrisGridTestUtils(dh);
+
+type TestUnderlyingModel = IrisGridModel & {
+  testUnderlyingMember: string;
+  testUnderlyingFunction: () => void;
+  get testUnderlyingGetter(): string;
+  set testUnderlyingSetter(value: string);
+  testMember: string;
+  testFunction: () => void;
+  get testGetter(): string;
+  set testSetter(value: string);
+};
+
+type TestProxyModel = IrisGridProxyModel & TestUnderlyingModel;
+
+describe('IrisGridProxyModel', () => {
+  let proxyModel: TestProxyModel;
+  let underlyingModel: TestUnderlyingModel;
+
+  beforeEach(() => {
+    proxyModel = irisGridTestUtils.makeModel() as TestProxyModel;
+    underlyingModel = proxyModel.model as TestUnderlyingModel;
+  });
+
+  // Getters must be set on prototype
+  test('Proxies getting members when necessary', () => {
+    Object.defineProperty(proxyModel, 'testMember', {
+      value: 'proxy',
+      writable: true,
+    });
+    Object.defineProperty(underlyingModel, 'testMember', {
+      value: 'underlying',
+      writable: true,
+    });
+    Object.defineProperty(underlyingModel, 'testUnderlyingMember', {
+      value: 'underlying',
+      writable: true,
+    });
+
+    expect(proxyModel.testMember).toBe('proxy');
+    expect(proxyModel.testUnderlyingMember).toBe('underlying');
+    expect(underlyingModel.testMember).toBe('underlying');
+  });
+
+  test('Proxies getters when necessary', () => {
+    const testGetter = jest.fn(() => 'proxy');
+    const testUnderlyingGetter = jest.fn(() => 'underlying');
+    Object.defineProperty(Object.getPrototypeOf(proxyModel), 'testGetter', {
+      get() {
+        return testGetter();
+      },
+    });
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testGetter',
+      {
+        get() {
+          return testUnderlyingGetter();
+        },
+      }
+    );
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testUnderlyingGetter',
+      {
+        get() {
+          return testUnderlyingGetter();
+        },
+      }
+    );
+
+    expect(proxyModel.testGetter).toBe('proxy');
+    expect(proxyModel.testUnderlyingGetter).toBe('underlying');
+    expect(underlyingModel.testGetter).toBe('underlying');
+    expect(testGetter).toHaveBeenCalledTimes(1);
+    expect(testUnderlyingGetter).toHaveBeenCalledTimes(2);
+  });
+
+  test('Proxies setting members when necessary', () => {
+    Object.defineProperty(proxyModel, 'testMember', {
+      value: 'proxy',
+      writable: true,
+    });
+    Object.defineProperty(underlyingModel, 'testMember', {
+      value: 'underlying',
+      writable: true,
+    });
+    Object.defineProperty(underlyingModel, 'testUnderlyingMember', {
+      value: 'underlying',
+      writable: true,
+    });
+
+    proxyModel.testMember = 'proxy2';
+    proxyModel.testUnderlyingMember = 'underlying2';
+
+    expect(
+      Object.getOwnPropertyDescriptor(proxyModel, 'testMember')?.value
+    ).toBe('proxy2');
+    expect(underlyingModel.testUnderlyingMember).toBe('underlying2');
+    expect(underlyingModel.testMember).toBe('underlying');
+  });
+
+  // Setters must be set on prototype
+  test('Proxies setters when necessary', () => {
+    const testSetter = jest.fn();
+    const testUnderlyingSetter = jest.fn();
+    Object.defineProperty(Object.getPrototypeOf(proxyModel), 'testSetter', {
+      set: testSetter,
+    });
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testSetter',
+      {
+        set: testUnderlyingSetter,
+      }
+    );
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testUnderlyingSetter',
+      {
+        set: testUnderlyingSetter,
+      }
+    );
+
+    proxyModel.testSetter = 'proxy';
+    proxyModel.testUnderlyingSetter = 'underlying';
+
+    expect(testSetter).toHaveBeenCalledWith('proxy');
+    expect(testSetter).toHaveBeenCalledTimes(1);
+    expect(testUnderlyingSetter).toHaveBeenCalledWith('underlying');
+    expect(testUnderlyingSetter).toHaveBeenCalledTimes(1);
+  });
+
+  // Functions must be set on prototype
+  test('Proxies functions when necessary', () => {
+    const testFn = jest.fn();
+    const testUnderlyingFn = jest.fn();
+    Object.defineProperty(Object.getPrototypeOf(proxyModel), 'testFunction', {
+      value: testFn,
+    });
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testFunction',
+      {
+        value: testUnderlyingFn,
+      }
+    );
+    Object.defineProperty(
+      Object.getPrototypeOf(underlyingModel),
+      'testUnderlyingFunction',
+      {
+        value: testUnderlyingFn,
+      }
+    );
+
+    proxyModel.testFunction();
+    proxyModel.testUnderlyingFunction();
+
+    expect(testFn).toHaveBeenCalledTimes(1);
+    expect(testUnderlyingFn).toHaveBeenCalledTimes(1);
+  });
+});

packages/iris-grid/src/IrisGridProxyModel.ts

@@ -129,7 +129,9 @@ class IrisGridProxyModel extends IrisGridModel implements PartitionedGridModel {
           Object.getOwnPropertyDescriptor(Object.getPrototypeOf(target), prop)
             ?.set != null;
 
-        if (proxyHasSetter) {
+        const proxyHasProp = Object.prototype.hasOwnProperty.call(target, prop);
+
+        if (proxyHasSetter || proxyHasProp) {
           return Reflect.set(target, prop, value, target);
         }