cleaning up proxys and getFieldRef hopefully to be more understandable and efficient

9 files changed, 97 insertions, 106 deletions

diff --git a/src/client/documents/Documents.ts b/src/client/documents/Documents.ts
index 39dde6fb4..a5190e66c 100644
--- a/src/client/documents/Documents.ts
+++ b/src/client/documents/Documents.ts
@@ -7,7 +7,6 @@ import { Id } from '../../fields/FieldSymbols';
 import { HtmlField } from '../../fields/HtmlField';
 import { InkField, PointData } from '../../fields/InkField';
 import { List } from '../../fields/List';
-import { ProxyField } from '../../fields/Proxy';
 import { RichTextField } from '../../fields/RichTextField';
 import { SchemaHeaderField } from '../../fields/SchemaHeaderField';
 import { ComputedField, ScriptField } from '../../fields/ScriptField';
@@ -673,8 +672,6 @@ export namespace Docs {
          * haven't been initialized, the newly initialized prototype document.
          */
         export async function initialize(): Promise<void> {
-            ProxyField.initPlugin();
-            ComputedField.initPlugin();
             // non-guid string ids for each document prototype
             const prototypeIds = Object.values(DocumentType)
                 .filter(type => type !== DocumentType.NONE)
diff --git a/src/client/util/SerializationHelper.ts b/src/client/util/SerializationHelper.ts
index 2d598c1ac..2d1f61cfb 100644
--- a/src/client/util/SerializationHelper.ts
+++ b/src/client/util/SerializationHelper.ts
@@ -1,6 +1,6 @@
-import { PropSchema, serialize, deserialize, custom, setDefaultModelSchema, getDefaultModelSchema } from "serializr";
-import { Field } from "../../fields/Doc";
-import { ClientUtils } from "./ClientUtils";
+import { PropSchema, serialize, deserialize, custom, setDefaultModelSchema, getDefaultModelSchema } from 'serializr';
+import { Field } from '../../fields/Doc';
+import { ClientUtils } from './ClientUtils';
 
 let serializing = 0;
 export function afterDocDeserialize(cb: (err: any, val: any) => void, err: any, newValue: any) {
@@ -25,7 +25,7 @@ export namespace SerializationHelper {
         serializing++;
         if (!(obj.constructor.name in reverseMap)) {
             serializing--;
-            throw Error("Error: " + `type '${obj.constructor.name}' not registered. Make sure you register it using a @Deserializable decorator`);
+            throw Error('Error: ' + `type '${obj.constructor.name}' not registered. Make sure you register it using a @Deserializable decorator`);
         }
 
         const json = serialize(obj);
@@ -59,24 +59,24 @@ export namespace SerializationHelper {
         const type = serializationTypes[obj.__type];
         const value = await new Promise(res => deserialize(type.ctor, obj, (err, result) => res(result)));
         if (type.afterDeserialize) {
-            await type.afterDeserialize(value);
+            type.afterDeserialize(value);
         }
         return value;
     }
 }
 
-const serializationTypes: { [name: string]: { ctor: { new(): any }, afterDeserialize?: (obj: any) => void | Promise<any> } } = {};
+const serializationTypes: { [name: string]: { ctor: { new (): any }; afterDeserialize?: (obj: any) => void | Promise<any> } } = {};
 const reverseMap: { [ctor: string]: string } = {};
 
 export interface DeserializableOpts {
-    (constructor: { new(...args: any[]): any }): void;
+    (constructor: { new (...args: any[]): any }): void;
     withFields(fields: string[]): Function;
 }
 
 export function Deserializable(name: string, afterDeserialize?: (obj: any) => void | Promise<any>): DeserializableOpts;
-export function Deserializable(constructor: { new(...args: any[]): any }): void;
-export function Deserializable(constructor: { new(...args: any[]): any } | string, afterDeserialize?: (obj: any) => void): DeserializableOpts | void {
-    function addToMap(name: string, ctor: { new(...args: any[]): any }) {
+export function Deserializable(constructor: { new (...args: any[]): any }): void;
+export function Deserializable(constructor: { new (...args: any[]): any } | string, afterDeserialize?: (obj: any) => void): DeserializableOpts | void {
+    function addToMap(name: string, ctor: { new (...args: any[]): any }) {
         const schema = getDefaultModelSchema(ctor) as any;
         if (schema.targetClass !== ctor) {
             const newSchema = { ...schema, factory: () => new ctor() };
@@ -89,17 +89,20 @@ export function Deserializable(constructor: { new(...args: any[]): any } | strin
             throw new Error(`Name ${name} has already been registered as deserializable`);
         }
     }
-    if (typeof constructor === "string") {
-        return Object.assign((ctor: { new(...args: any[]): any }) => {
-            addToMap(constructor, ctor);
-        }, { withFields: (fields: string[]) => Deserializable.withFields(fields, constructor, afterDeserialize) });
+    if (typeof constructor === 'string') {
+        return Object.assign(
+            (ctor: { new (...args: any[]): any }) => {
+                addToMap(constructor, ctor);
+            },
+            { withFields: (fields: string[]) => Deserializable.withFields(fields, constructor, afterDeserialize) }
+        );
     }
     addToMap(constructor.name, constructor);
 }
 
 export namespace Deserializable {
     export function withFields(fields: string[], name?: string, afterDeserialize?: (obj: any) => void | Promise<any>) {
-        return function (constructor: { new(...fields: any[]): any }) {
+        return function (constructor: { new (...fields: any[]): any }) {
             Deserializable(name || constructor.name, afterDeserialize)(constructor);
             let schema = getDefaultModelSchema(constructor);
             if (schema) {
@@ -128,7 +131,7 @@ export namespace Deserializable {
                     factory: context => {
                         const args = fields.map(key => context.json[key]);
                         return new constructor(...args);
-                    }
+                    },
                 };
                 setDefaultModelSchema(constructor, schema);
             }
@@ -138,7 +141,7 @@ export namespace Deserializable {
 
 export function autoObject(): PropSchema {
     return custom(
-        (s) => SerializationHelper.Serialize(s),
+        s => SerializationHelper.Serialize(s),
         (json: any, context: any, oldValue: any, cb: (err: any, result: any) => void) => SerializationHelper.Deserialize(json).then(res => cb(null, res))
     );
-}
\ No newline at end of file
+}
diff --git a/src/client/util/SharingManager.tsx b/src/client/util/SharingManager.tsx
index 54ee691bc..ae4524b5e 100644
--- a/src/client/util/SharingManager.tsx
+++ b/src/client/util/SharingManager.tsx
@@ -290,7 +290,11 @@ export class SharingManager extends React.Component<{}> {
                 doc.backgroundColor = 'green';
             } else {
                 const acls = doc[DataSym][AclSym];
-                if (Object.keys(acls).every(key => (key === `acl-${Doc.CurrentUserEmailNormalized}` || key === 'acl-Me' ? true : [AclUnset, AclPrivate].includes(acls[key])))) {
+                if (
+                    Object.keys(acls)
+                        .filter(key => key !== `acl-${Doc.CurrentUserEmailNormalized}` && key !== 'acl-Me')
+                        .every(key => [AclUnset, AclPrivate].includes(acls[key]))
+                ) {
                     doc.isShared = undefined;
                     doc.backgroundColor = undefined;
                 }
diff --git a/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx b/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
index d59cec02d..e8f382251 100644
--- a/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
+++ b/src/client/views/collections/collectionFreeForm/CollectionFreeFormView.tsx
@@ -1605,12 +1605,14 @@ export class CollectionFreeFormView extends CollectionSubView<Partial<collection
                                 r: cbounds.r - p[0] + c[0],
                                 b: cbounds.b - p[1] + c[1],
                             };
-                            this.layoutDoc._width = pbounds.r - pbounds.x;
-                            this.layoutDoc._height = pbounds.b - pbounds.y;
-                            this.layoutDoc._panX = (cbounds.r + cbounds.x) / 2;
-                            this.layoutDoc._panY = (cbounds.b + cbounds.y) / 2;
-                            this.layoutDoc.x = pbounds.x;
-                            this.layoutDoc.y = pbounds.y;
+                            if (Number.isFinite(pbounds.r - pbounds.x) && Number.isFinite(pbounds.b - pbounds.y)) {
+                                this.layoutDoc._width = pbounds.r - pbounds.x;
+                                this.layoutDoc._height = pbounds.b - pbounds.y;
+                                this.layoutDoc._panX = (cbounds.r + cbounds.x) / 2;
+                                this.layoutDoc._panY = (cbounds.b + cbounds.y) / 2;
+                                this.layoutDoc.x = pbounds.x;
+                                this.layoutDoc.y = pbounds.y;
+                            }
                         }
                     },
                     { fireImmediately: true }
diff --git a/src/fields/Doc.ts b/src/fields/Doc.ts
index 51313a8df..6762665a2 100644
--- a/src/fields/Doc.ts
+++ b/src/fields/Doc.ts
@@ -276,7 +276,7 @@ export class Doc extends RefField {
     }
     constructor(id?: FieldId, forceSave?: boolean) {
         super(id);
-        const doc = new Proxy<this>(this, {
+        const docProxy = new Proxy<this>(this, {
             set: setter,
             get: getter,
             // getPrototypeOf: (target) => Cast(target[SelfProxy].proto, Doc) || null, // TODO this might be able to replace the proto logic in getter
@@ -305,11 +305,11 @@ export class Doc extends RefField {
                 throw new Error("Currently properties can't be defined on documents using Object.defineProperty");
             },
         });
-        this[SelfProxy] = doc;
+        this[SelfProxy] = docProxy;
         if (!id || forceSave) {
-            DocServer.CreateField(doc);
+            DocServer.CreateField(docProxy);
         }
-        return doc;
+        return docProxy;
     }
 
     proto: Opt<Doc>;
diff --git a/src/fields/FieldSymbols.ts b/src/fields/FieldSymbols.ts
index 8d040f493..e50c2856f 100644
--- a/src/fields/FieldSymbols.ts
+++ b/src/fields/FieldSymbols.ts
@@ -1,12 +1,12 @@
-
-export const Update = Symbol("Update");
-export const Self = Symbol("Self");
-export const SelfProxy = Symbol("SelfProxy");
-export const HandleUpdate = Symbol("HandleUpdate");
-export const Id = Symbol("Id");
-export const OnUpdate = Symbol("OnUpdate");
-export const Parent = Symbol("Parent");
-export const Copy = Symbol("Copy");
-export const ToScriptString = Symbol("ToScriptString");
-export const ToPlainText = Symbol("ToPlainText");
-export const ToString = Symbol("ToString");
+export const Update = Symbol('Update');
+export const Self = Symbol('Self');
+export const SelfProxy = Symbol('SelfProxy');
+export const HandleUpdate = Symbol('HandleUpdate');
+export const Id = Symbol('Id');
+export const OnUpdate = Symbol('OnUpdate');
+export const Parent = Symbol('Parent');
+export const Copy = Symbol('Copy');
+export const ToValue = Symbol('ToValue');
+export const ToScriptString = Symbol('ToScriptString');
+export const ToPlainText = Symbol('ToPlainText');
+export const ToString = Symbol('ToString');
diff --git a/src/fields/Proxy.ts b/src/fields/Proxy.ts
index 1266e6e3c..55d1d9ea4 100644
--- a/src/fields/Proxy.ts
+++ b/src/fields/Proxy.ts
@@ -5,9 +5,8 @@ import { observable, action, runInAction, computed } from 'mobx';
 import { DocServer } from '../client/DocServer';
 import { RefField } from './RefField';
 import { ObjectField } from './ObjectField';
-import { Id, Copy, ToScriptString, ToString } from './FieldSymbols';
+import { Id, Copy, ToScriptString, ToString, ToValue } from './FieldSymbols';
 import { scriptingGlobal } from '../client/util/ScriptingGlobals';
-import { Plugins } from './util';
 
 function deserializeProxy(field: any) {
     if (!field.cache.field) {
@@ -30,6 +29,10 @@ export class ProxyField<T extends RefField> extends ObjectField {
         }
     }
 
+    [ToValue](doc: any) {
+        return ProxyField.toValue(this);
+    }
+
     [Copy]() {
         if (this.cache.field) return new ProxyField<T>(this.cache.field);
         return new ProxyField<T>(this.fieldId);
@@ -105,12 +108,10 @@ export namespace ProxyField {
         }
     }
 
-    export function initPlugin() {
-        Plugins.addGetterPlugin((doc, _, value) => {
-            if (useProxy && value instanceof ProxyField) {
-                return { value: value.value };
-            }
-        });
+    export function toValue(value: any) {
+        if (useProxy) {
+            return { value: value.value };
+        }
     }
 }
 
diff --git a/src/fields/ScriptField.ts b/src/fields/ScriptField.ts
index 4896c027d..b23732b45 100644
--- a/src/fields/ScriptField.ts
+++ b/src/fields/ScriptField.ts
@@ -6,11 +6,10 @@ import { scriptingGlobal, ScriptingGlobals } from '../client/util/ScriptingGloba
 import { autoObject, Deserializable } from '../client/util/SerializationHelper';
 import { numberRange } from '../Utils';
 import { Doc, Field, Opt } from './Doc';
-import { Copy, Id, ToScriptString, ToString } from './FieldSymbols';
+import { Copy, Id, ToScriptString, ToString, ToValue } from './FieldSymbols';
 import { List } from './List';
 import { ObjectField } from './ObjectField';
 import { Cast, StrCast } from './Types';
-import { Plugins } from './util';
 
 function optional(propSchema: PropSchema) {
     return custom(
@@ -175,6 +174,9 @@ export class ComputedField extends ScriptField {
     value = computedFn((doc: Doc) => this._valueOutsideReaction(doc));
     _valueOutsideReaction = (doc: Doc) => (this._lastComputedResult = this.script.run({ this: doc, self: Cast(doc.rootDocument, Doc, null) || doc, _last_: this._lastComputedResult, _readOnly_: true }, console.log).result);
 
+    [ToValue](doc: Doc) {
+        return ComputedField.toValue(doc, this);
+    }
     [Copy](): ObjectField {
         return new ComputedField(this.script, this.setterscript, this.rawscript);
     }
@@ -239,12 +241,10 @@ export namespace ComputedField {
         }
     }
 
-    export function initPlugin() {
-        Plugins.addGetterPlugin((doc, _, value) => {
-            if (useComputed && value instanceof ComputedField) {
-                return { value: value._valueOutsideReaction(doc), shouldReturn: true };
-            }
-        });
+    export function toValue(doc: any, value: any) {
+        if (useComputed) {
+            return { value: value._valueOutsideReaction(doc) };
+        }
     }
 }
 
diff --git a/src/fields/util.ts b/src/fields/util.ts
index 51c76b19a..285cbb4c6 100644
--- a/src/fields/util.ts
+++ b/src/fields/util.ts
@@ -1,4 +1,4 @@
-import { action, observable, runInAction, trace } from 'mobx';
+import { $mobx, action, observable, runInAction, trace } from 'mobx';
 import { computedFn } from 'mobx-utils';
 import { DocServer } from '../client/DocServer';
 import { CollectionViewType } from '../client/documents/DocumentTypes';
@@ -28,7 +28,7 @@ import {
     UpdatingFromServer,
     WidthSym,
 } from './Doc';
-import { Id, OnUpdate, Parent, Self, SelfProxy, Update } from './FieldSymbols';
+import { Id, OnUpdate, Parent, SelfProxy, ToValue, Update } from './FieldSymbols';
 import { List } from './List';
 import { ObjectField } from './ObjectField';
 import { PrefetchProxy, ProxyField } from './Proxy';
@@ -47,19 +47,6 @@ export function TraceMobx() {
     tracing && trace();
 }
 
-export interface GetterResult {
-    value: FieldResult;
-    shouldReturn?: boolean;
-}
-export type GetterPlugin = (receiver: any, prop: string | number, currentValue: any) => GetterResult | undefined;
-const getterPlugins: GetterPlugin[] = [];
-
-export namespace Plugins {
-    export function addGetterPlugin(plugin: GetterPlugin) {
-        getterPlugins.push(plugin);
-    }
-}
-
 const _setterImpl = action(function (target: any, prop: string | symbol | number, value: any, receiver: any): boolean {
     if (SerializationHelper.IsSerializing()) {
         target[prop] = value;
@@ -353,47 +340,44 @@ export function setter(target: any, in_prop: string | symbol | number, value: an
     return _setter(target, prop, value, receiver);
 }
 
-export function getter(target: any, in_prop: string | symbol, receiver: any): any {
-    if (in_prop === 'constructor' || in_prop === 'toString' || in_prop === 'valueOf' || in_prop === 'factory' || in_prop === 'serializeInfo') return target[in_prop];
-    if (in_prop === 'then') return undefined; //If we're being awaited
-    if (in_prop === AclSym) return target[AclSym];
-    if (in_prop === LayoutSym) return target.__LAYOUT__;
-    if ((in_prop === HeightSym || in_prop === WidthSym) && GetEffectiveAcl(target) === AclPrivate) return returnZero;
-    if (typeof in_prop === 'symbol' || in_prop.startsWith('isMobX') || in_prop.startsWith('__')) return target.__fields[in_prop] || target[in_prop];
-    if (GetEffectiveAcl(target) === AclPrivate) {
-        if (in_prop === 'author') return target.__fields[in_prop] || target[in_prop];
-        return undefined;
+export function getter(target: any, prop: string | symbol, proxy: any): any {
+    // prettier-ignore
+    switch (prop) {
+        case 'then' :   return undefined;
+        case '__fields' :   case '__id':  
+        case 'constructor': case 'toString': case 'valueOf':  
+        case 'factory':     case 'serializeInfo': 
+                        return target[prop];
+        case AclSym :   return target[AclSym];
+        case $mobx:     return target.__fields[prop];
+        case LayoutSym: return target.__Layout__;
+        case HeightSym: case WidthSym: if (GetEffectiveAcl(target) === AclPrivate) return returnZero;
+        default :
+            if (typeof prop === 'symbol')  return target[prop];
+            if (prop.startsWith('isMobX')) return target[prop];
+            if (prop.startsWith('__'))     return target[prop];
+            if (GetEffectiveAcl(target) === AclPrivate && prop !== 'author') return undefined;
     }
 
-    const prop = in_prop.startsWith('_') ? in_prop.substring(1) : in_prop;
-    if (prop !== in_prop && target.__LAYOUT__) return target.__LAYOUT__[prop];
-    if (SerializationHelper.IsSerializing()) return target[prop];
-    return getFieldImpl(target, prop, receiver);
+    const layout_prop = prop.startsWith('_') ? prop.substring(1) : undefined;
+    if (layout_prop && target.__LAYOUT__) return target.__LAYOUT__[layout_prop];
+    return getFieldImpl(target, layout_prop ?? prop, proxy);
 }
 
-function getFieldImpl(target: any, prop: string | number, receiver: any, ignoreProto: boolean = false): any {
-    receiver = receiver || target[SelfProxy];
-    let field = target.__fields[prop];
-    for (const plugin of getterPlugins) {
-        const res = plugin(receiver, prop, field);
-        if (res === undefined) continue;
-        if (res.shouldReturn) {
-            return res.value;
-        } else {
-            field = res.value;
-        }
-    }
-    if (field === undefined && !ignoreProto && prop !== 'proto') {
-        const proto = getFieldImpl(target, 'proto', receiver, true); //TODO tfs: instead of receiver we could use target[SelfProxy]... I don't which semantics we want or if it really matters
+function getFieldImpl(target: any, prop: string | number, proxy: any, ignoreProto: boolean = false): any {
+    const field = target.__fields[prop];
+    const value = field?.[ToValue]?.(proxy); // converts ComputedFields to values, or unpacks ProxyFields into Proxys
+    if (value) return value.value;
+    if (!field && !ignoreProto && prop !== 'proto') {
+        const proto = getFieldImpl(target, 'proto', proxy, true); //TODO tfs: instead of proxy we could use target[SelfProxy]... I don't which semantics we want or if it really matters
         if (proto instanceof Doc && GetEffectiveAcl(proto) !== AclPrivate) {
-            return getFieldImpl(proto[Self], prop, receiver, ignoreProto);
+            return getFieldImpl(proto, prop, proxy, ignoreProto);
         }
-        return undefined;
     }
     return field;
 }
 export function getField(target: any, prop: string | number, ignoreProto: boolean = false): any {
-    return getFieldImpl(target, prop, undefined, ignoreProto);
+    return getFieldImpl(target, prop, target[SelfProxy], ignoreProto);
 }
 
 export function deleteProperty(target: any, prop: string | number | symbol) {