fixed document leak related to capturedVariables in scripts. changed capturedVariables to be a string encoding the captured variables instead of a Doc.

1 files changed, 96 insertions, 73 deletions

diff --git a/src/fields/ScriptField.ts b/src/fields/ScriptField.ts
index 40ca0ce22..2b4b1ef4c 100644
--- a/src/fields/ScriptField.ts
+++ b/src/fields/ScriptField.ts
@@ -1,29 +1,32 @@
-import { computedFn } from "mobx-utils";
-import { createSimpleSchema, custom, map, object, primitive, PropSchema, serializable, SKIP } from "serializr";
-import { CompiledScript, CompileScript } from "../client/util/Scripting";
-import { scriptingGlobal, ScriptingGlobals } from "../client/util/ScriptingGlobals";
-import { autoObject, Deserializable } from "../client/util/SerializationHelper";
-import { numberRange } from "../Utils";
-import { Doc, Field, Opt } from "./Doc";
-import { Copy, ToScriptString, ToString } from "./FieldSymbols";
-import { List } from "./List";
-import { ObjectField } from "./ObjectField";
-import { ProxyField } from "./Proxy";
-import { Cast, NumCast } from "./Types";
-import { Plugins } from "./util";
+import { computedFn } from 'mobx-utils';
+import { createSimpleSchema, custom, map, object, primitive, PropSchema, serializable, SKIP } from 'serializr';
+import { DocServer } from '../client/DocServer';
+import { CompiledScript, CompileScript, ScriptOptions } from '../client/util/Scripting';
+import { scriptingGlobal, ScriptingGlobals } from '../client/util/ScriptingGlobals';
+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 { List } from './List';
+import { ObjectField } from './ObjectField';
+import { Cast, NumCast } from './Types';
+import { Plugins } from './util';
 
 function optional(propSchema: PropSchema) {
-    return custom(value => {
-        if (value !== undefined) {
-            return propSchema.serializer(value);
-        }
-        return SKIP;
-    }, (jsonValue: any, context: any, oldValue: any, callback: (err: any, result: any) => void) => {
-        if (jsonValue !== undefined) {
-            return propSchema.deserializer(jsonValue, callback, context, oldValue);
+    return custom(
+        value => {
+            if (value !== undefined) {
+                return propSchema.serializer(value);
+            }
+            return SKIP;
+        },
+        (jsonValue: any, context: any, oldValue: any, callback: (err: any, result: any) => void) => {
+            if (jsonValue !== undefined) {
+                return propSchema.deserializer(jsonValue, callback, context, oldValue);
+            }
+            return SKIP;
         }
-        return SKIP;
-    });
+    );
 }
 
 const optionsSchema = createSimpleSchema({
@@ -32,31 +35,19 @@ const optionsSchema = createSimpleSchema({
     typecheck: true,
     editable: true,
     readonly: true,
-    params: optional(map(primitive()))
+    params: optional(map(primitive())),
 });
 
 const scriptSchema = createSimpleSchema({
     options: object(optionsSchema),
-    originalScript: true
+    originalScript: true,
 });
 
-async function deserializeScript(script: ScriptField) {
-    const captures: ProxyField<Doc> = (script as any).captures;
-    const cache = captures ? undefined : ScriptField.GetScriptFieldCache(script.script.originalScript);
-    if (cache) return (script as any).script = cache;
-    if (captures) {
-        const doc = (await captures.value())!;
-        const captured: any = {};
-        const keys = Object.keys(doc);
-        const vals = await Promise.all(keys.map(key => doc[key]) as any);
-        keys.forEach((key, i) => captured[key] = vals[i]);
-        (script.script.options as any).capturedVariables = captured;
-    }
+function finalizeScript(script: ScriptField, captures: boolean) {
     const comp = CompileScript(script.script.originalScript, script.script.options);
     if (!comp.compiled) {
         throw new Error("Couldn't compile loaded script");
     }
-    (script as any).script = comp;
     !captures && ScriptField._scriptFieldCache.set(script.script.originalScript, comp);
     if (script.setterscript) {
         const compset = CompileScript(script.setterscript?.originalScript, script.setterscript.options);
@@ -65,10 +56,30 @@ async function deserializeScript(script: ScriptField) {
         }
         (script as any).setterscript = compset;
     }
+    return comp;
+}
+async function deserializeScript(script: ScriptField) {
+    if (script.captures) {
+        const captured: any = {};
+        (script.script.options as ScriptOptions).capturedVariables = captured;
+        Promise.all(
+            script.captures.map(async capture => {
+                const key = capture.split(':')[0];
+                const val = capture.split(':')[1];
+                if (val === 'true') captured[key] = true;
+                else if (val === 'false') captured[key] = false;
+                else if (val.startsWith('ID->')) captured[key] = await DocServer.GetRefField(val.replace('ID->', ''));
+                else if (!isNaN(Number(val))) captured[key] = Number(val);
+                else captured[key] = val;
+            })
+        ).then(() => ((script as any).script = finalizeScript(script, true)));
+    } else {
+        (script as any).script = ScriptField.GetScriptFieldCache(script.script.originalScript) ?? finalizeScript(script, false);
+    }
 }
 
 @scriptingGlobal
-@Deserializable("script", deserializeScript)
+@Deserializable('script', deserializeScript)
 export class ScriptField extends ObjectField {
     @serializable(object(scriptSchema))
     readonly script: CompiledScript;
@@ -76,18 +87,19 @@ export class ScriptField extends ObjectField {
     readonly setterscript: CompiledScript | undefined;
 
     @serializable(autoObject())
-    private captures?: ProxyField<Doc>;
+    captures?: List<string>;
 
     public static _scriptFieldCache: Map<string, Opt<CompiledScript>> = new Map();
-    public static GetScriptFieldCache(field: string) { return this._scriptFieldCache.get(field); }
+    public static GetScriptFieldCache(field: string) {
+        return this._scriptFieldCache.get(field);
+    }
 
     constructor(script: CompiledScript, setterscript?: CompiledScript) {
         super();
 
-        if (script?.options.capturedVariables) {
-            const doc = Doc.assign(new Doc, script.options.capturedVariables);
-            doc.system = true;
-            this.captures = new ProxyField(doc);
+        const captured = script?.options.capturedVariables;
+        if (captured) {
+            this.captures = new List<string>(Object.keys(captured).map(key => key + ':' + (captured[key] instanceof Doc ? 'ID->' + (captured[key] as Doc)[Id] : captured[key].toString())));
         }
         this.setterscript = setterscript;
         this.script = script;
@@ -122,46 +134,45 @@ export class ScriptField extends ObjectField {
     }
 
     [ToScriptString]() {
-        return "script field";
+        return 'script field';
     }
     [ToString]() {
         return this.script.originalScript;
     }
-    public static CompileScript(script: string, params: object = {}, addReturn = false, capturedVariables?: { [name: string]: Field }) {
+    public static CompileScript(script: string, params: object = {}, addReturn = false, capturedVariables?: { [name: string]: Doc | string | number | boolean }) {
         const compiled = CompileScript(script, {
             params: {
-                this: Doc?.name || "Doc",  // this is the doc that executes the script
-                self: Doc?.name || "Doc",  // self is the root doc of the doc that executes the script
-                _last_: "any",             // _last_ is the previous value of a computed field when it is being triggered to re-run.
-                _readOnly_: "boolean",     // _readOnly_ is set when a computed field is executed to indicate that it should not have mobx side-effects.  used for checking the value of a set function (see FontIconBox)
-                ...params
+                this: Doc?.name || 'Doc', // this is the doc that executes the script
+                self: Doc?.name || 'Doc', // self is the root doc of the doc that executes the script
+                _last_: 'any', // _last_ is the previous value of a computed field when it is being triggered to re-run.
+                _readOnly_: 'boolean', // _readOnly_ is set when a computed field is executed to indicate that it should not have mobx side-effects.  used for checking the value of a set function (see FontIconBox)
+                ...params,
             },
             typecheck: false,
             editable: true,
             addReturn: addReturn,
-            capturedVariables
+            capturedVariables,
         });
         return compiled;
     }
-    public static MakeFunction(script: string, params: object = {}, capturedVariables?: { [name: string]: Field }) {
+    public static MakeFunction(script: string, params: object = {}, capturedVariables?: { [name: string]: Doc | string | number | boolean }) {
         const compiled = ScriptField.CompileScript(script, params, true, capturedVariables);
         return compiled.compiled ? new ScriptField(compiled) : undefined;
     }
 
-    public static MakeScript(script: string, params: object = {}, capturedVariables?: { [name: string]: Field }) {
+    public static MakeScript(script: string, params: object = {}, capturedVariables?: { [name: string]: Doc | string | number | boolean }) {
         const compiled = ScriptField.CompileScript(script, params, false, capturedVariables);
         return compiled.compiled ? new ScriptField(compiled) : undefined;
     }
 }
 
 @scriptingGlobal
-@Deserializable("computed", deserializeScript)
+@Deserializable('computed', deserializeScript)
 export class ComputedField extends ScriptField {
     _lastComputedResult: any;
     //TODO maybe add an observable cache based on what is passed in for doc, considering there shouldn't really be that many possible values for doc
     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;
-
+    _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);
 
     [Copy](): ObjectField {
         return new ComputedField(this.script, this.setterscript);
@@ -171,7 +182,7 @@ export class ComputedField extends ScriptField {
         const compiled = ScriptField.CompileScript(script, params, false);
         return compiled.compiled ? new ComputedField(compiled) : undefined;
     }
-    public static MakeFunction(script: string, params: object = {}, capturedVariables?: { [name: string]: Field }) {
+    public static MakeFunction(script: string, params: object = {}, capturedVariables?: { [name: string]: Doc | string | number | boolean }) {
         const compiled = ScriptField.CompileScript(script, params, true, capturedVariables);
         return compiled.compiled ? new ComputedField(compiled) : undefined;
     }
@@ -182,7 +193,7 @@ export class ComputedField extends ScriptField {
             doc[`${fieldKey}-indexed`] = flist;
         }
         const getField = ScriptField.CompileScript(`getIndexVal(self['${fieldKey}-indexed'], self.${interpolatorKey})`, {}, true, {});
-        const setField = ScriptField.CompileScript(`setIndexVal(self['${fieldKey}-indexed'], self.${interpolatorKey}, value)`, { value: "any" }, true, {});
+        const setField = ScriptField.CompileScript(`setIndexVal(self['${fieldKey}-indexed'], self.${interpolatorKey}, value)`, { value: 'any' }, true, {});
         return getField.compiled ? new ComputedField(getField, setField?.compiled ? setField : undefined) : undefined;
     }
 }
@@ -196,7 +207,7 @@ export namespace ComputedField {
         useComputed = true;
     }
 
-    export const undefined = "__undefined";
+    export const undefined = '__undefined';
 
     export function WithoutComputed<T>(fn: () => T) {
         DisableComputedFields();
@@ -216,15 +227,27 @@ export namespace ComputedField {
     }
 }
 
-ScriptingGlobals.add(function setIndexVal(list: any[], index: number, value: any) {
-    while (list.length <= index) list.push(undefined);
-    list[index] = value;
-}, "sets the value at a given index of a list", "(list: any[], index: number, value: any)");
-
-ScriptingGlobals.add(function getIndexVal(list: any[], index: number) {
-    return list?.reduce((p, x, i) => (i <= index && x !== undefined) || p === undefined ? x : p, undefined as any);
-}, "returns the value at a given index of a list", "(list: any[], index: number)");
-
-ScriptingGlobals.add(function makeScript(script: string) {
-    return ScriptField.MakeScript(script);
-}, "returns the value at a given index of a list", "(list: any[], index: number)");
+ScriptingGlobals.add(
+    function setIndexVal(list: any[], index: number, value: any) {
+        while (list.length <= index) list.push(undefined);
+        list[index] = value;
+    },
+    'sets the value at a given index of a list',
+    '(list: any[], index: number, value: any)'
+);
+
+ScriptingGlobals.add(
+    function getIndexVal(list: any[], index: number) {
+        return list?.reduce((p, x, i) => ((i <= index && x !== undefined) || p === undefined ? x : p), undefined as any);
+    },
+    'returns the value at a given index of a list',
+    '(list: any[], index: number)'
+);
+
+ScriptingGlobals.add(
+    function makeScript(script: string) {
+        return ScriptField.MakeScript(script);
+    },
+    'returns the value at a given index of a list',
+    '(list: any[], index: number)'
+);