From 53bcd3dda6a909aac669757b69b074403ee9895a Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Sat, 5 Feb 2022 22:43:27 +0100 Subject: [PATCH 1/8] WIP hooks refactor --- packages/engine/src/executor.ts | 24 ++++++--- packages/engine/src/hookDisposables.ts | 67 +++++++++++++++++++++----- 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 478705fd9..e4c57fbea 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1,6 +1,6 @@ import { autorun, runInAction } from "mobx"; import { TypeCellContext } from "./context"; -import { installHooks } from "./hookDisposables"; +import { executeWithHooks, installHooks } from "./hookDisposables"; import { Module } from "./modules"; import { isStored } from "./storage/stored"; import { isView } from "./view"; @@ -114,18 +114,26 @@ export async function runModule( disposeEveryRun.length = 0; // clear existing array in this way, because we've passed the reference to resolveDependencyArray and want to keep it intact beforeExecuting(); - const hooks = installHooks(); - disposeEveryRun.push(hooks.disposeAll); + + console.log(disposeEveryRun); + let executionPromise: Promise; + let disposers: Array<() => void> = []; + + const { execute } = executeWithHooks( + () => mod.factoryFunction.apply(undefined, argsToCallFunctionWith), // TODO: what happens with disposers if a rerun of this function is slow / delayed? + disposers + ); + + disposeEveryRun.push(() => { + disposers.forEach((d) => d()); + }); + try { - executionPromise = mod.factoryFunction.apply( - undefined, - argsToCallFunctionWith - ); // TODO: what happens with disposers if a rerun of this function is slow / delayed? + executionPromise = execute(); } finally { // Hooks are only installed for sync code. Ideally, we'd want to run it for all code, but then we have the chance hooks will affect other parts of the TypeCell (non-user) code // (we ran into this that notebooks wouldn't be saved (_.debounce), and also that setTimeout of Monaco blink cursor would be hooked) - hooks.unHookAll(); if (previousVariableDisposer) { previousVariableDisposer(exports); } diff --git a/packages/engine/src/hookDisposables.ts b/packages/engine/src/hookDisposables.ts index efb68a2c3..f134af72e 100644 --- a/packages/engine/src/hookDisposables.ts +++ b/packages/engine/src/hookDisposables.ts @@ -4,9 +4,60 @@ type FunctionPropertyNames = { type Hook = { disposeAll: () => void; - unHook: () => void; }; +const glob = typeof window === "undefined" ? global : window; + +function globalFunctionOverride( + method: string, + disposes: Array<() => void>, + disposer: (ret: any, args: IArguments) => void +) { + const originalFunction = (glob as any)[method]; + + return function (this: any) { + const args = arguments; + const ret = (originalFunction as any).apply(this, args); // TODO: fix any? + const ctx = this; + disposes.push(() => disposer.call(ctx, ret, args)); + return ret; + }; +} + +export function executeWithHooks( + execution: () => T, + disposers: Array<() => void> +) { + const execute = () => { + // manipulate execution scope + (setTimeout as any) = globalFunctionOverride( + "setTimeout", + disposers, + (ret) => { + console.log("DISPOSING"); + clearTimeout(ret as any); + } + ); + (setInterval as any) = globalFunctionOverride( + "setInterval", + disposers, + (ret) => { + console.log("DISPOSING interval yo"); + clearInterval(ret as any); + } + ); + + const executionPromise = execution(); + return executionPromise; + }; + + console.log(disposers); + + return { + execute, + }; +} + function installHook>( obj: T, method: K, @@ -27,19 +78,16 @@ function installHook>( disposeAll: () => { disposes.forEach((d) => d()); }, - unHook: () => { - obj[method] = originalFunction; - }, }; } -const wnd = typeof window === "undefined" ? global : window; - export function installHooks() { const hooks: Hook[] = []; - hooks.push(installHook(wnd, "setTimeout", (ret) => clearTimeout(ret as any))); hooks.push( - installHook(wnd, "setInterval", (ret) => clearInterval(ret as any)) + installHook(glob, "setTimeout", (ret) => clearTimeout(ret as any)) + ); + hooks.push( + installHook(glob, "setInterval", (ret) => clearInterval(ret as any)) ); if (typeof EventTarget !== "undefined") { @@ -58,8 +106,5 @@ export function installHooks() { disposeAll: () => { hooks.forEach((h) => h.disposeAll()); }, - unHookAll: () => { - hooks.forEach((h) => h.unHook()); - }, }; } From 9f550e728b21a33846378940f185ea90aaf69d21 Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Sun, 6 Feb 2022 21:05:48 +0100 Subject: [PATCH 2/8] Refactor hooks --- packages/engine/src/executor.ts | 27 ++---- packages/engine/src/hookDisposables.ts | 110 ------------------------- packages/engine/src/hooks.ts | 72 ++++++++++++++++ packages/engine/src/modules.ts | 12 +++ 4 files changed, 92 insertions(+), 129 deletions(-) delete mode 100644 packages/engine/src/hookDisposables.ts create mode 100644 packages/engine/src/hooks.ts diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index e4c57fbea..34fd91848 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1,6 +1,6 @@ import { autorun, runInAction } from "mobx"; import { TypeCellContext } from "./context"; -import { executeWithHooks, installHooks } from "./hookDisposables"; +import { executeWithHooks } from "./hooks"; import { Module } from "./modules"; import { isStored } from "./storage/stored"; import { isView } from "./view"; @@ -96,7 +96,7 @@ export async function runModule( ); } - const execute = async () => { + async function execute(this: any) { try { if (wouldLoopOnAutorun) { detectedLoop = true; @@ -115,32 +115,21 @@ export async function runModule( beforeExecuting(); - console.log(disposeEveryRun); - - let executionPromise: Promise; - let disposers: Array<() => void> = []; - - const { execute } = executeWithHooks( - () => mod.factoryFunction.apply(undefined, argsToCallFunctionWith), // TODO: what happens with disposers if a rerun of this function is slow / delayed? - disposers + const { executeModel, disposeHooks } = executeWithHooks( + (hookContext) => + mod.factoryFunction.apply(hookContext, argsToCallFunctionWith) // TODO: what happens with disposers if a rerun of this function is slow / delayed? ); - disposeEveryRun.push(() => { - disposers.forEach((d) => d()); - }); + disposeEveryRun.push(disposeHooks); try { - executionPromise = execute(); + await executeModel(); } finally { - // Hooks are only installed for sync code. Ideally, we'd want to run it for all code, but then we have the chance hooks will affect other parts of the TypeCell (non-user) code - // (we ran into this that notebooks wouldn't be saved (_.debounce), and also that setTimeout of Monaco blink cursor would be hooked) if (previousVariableDisposer) { previousVariableDisposer(exports); } } - await executionPromise; - // Running the assignments to `context` in action should be a performance improvement to prevent triggering observers one-by-one wouldLoopOnAutorun = true; runInAction(() => { @@ -219,7 +208,7 @@ export async function runModule( //reject(e); resolve(); } - }; + } const autorunDisposer = autorun(() => execute()); diff --git a/packages/engine/src/hookDisposables.ts b/packages/engine/src/hookDisposables.ts deleted file mode 100644 index f134af72e..000000000 --- a/packages/engine/src/hookDisposables.ts +++ /dev/null @@ -1,110 +0,0 @@ -type FunctionPropertyNames = { - [K in keyof T]: T[K] extends Function ? K : never; -}[keyof T]; - -type Hook = { - disposeAll: () => void; -}; - -const glob = typeof window === "undefined" ? global : window; - -function globalFunctionOverride( - method: string, - disposes: Array<() => void>, - disposer: (ret: any, args: IArguments) => void -) { - const originalFunction = (glob as any)[method]; - - return function (this: any) { - const args = arguments; - const ret = (originalFunction as any).apply(this, args); // TODO: fix any? - const ctx = this; - disposes.push(() => disposer.call(ctx, ret, args)); - return ret; - }; -} - -export function executeWithHooks( - execution: () => T, - disposers: Array<() => void> -) { - const execute = () => { - // manipulate execution scope - (setTimeout as any) = globalFunctionOverride( - "setTimeout", - disposers, - (ret) => { - console.log("DISPOSING"); - clearTimeout(ret as any); - } - ); - (setInterval as any) = globalFunctionOverride( - "setInterval", - disposers, - (ret) => { - console.log("DISPOSING interval yo"); - clearInterval(ret as any); - } - ); - - const executionPromise = execution(); - return executionPromise; - }; - - console.log(disposers); - - return { - execute, - }; -} - -function installHook>( - obj: T, - method: K, - disposeSingle: (ret: ReturnType, args: IArguments) => void -): Hook { - const disposes: Array<() => void> = []; - - const originalFunction = obj[method]; - (obj[method] as any) = function (this: any) { - const args = arguments; - const ret = (originalFunction as any).apply(this, args); // TODO: fix any? - const ctx = this; - disposes.push(() => disposeSingle.call(ctx, ret, args)); - return ret; - }; - - return { - disposeAll: () => { - disposes.forEach((d) => d()); - }, - }; -} - -export function installHooks() { - const hooks: Hook[] = []; - hooks.push( - installHook(glob, "setTimeout", (ret) => clearTimeout(ret as any)) - ); - hooks.push( - installHook(glob, "setInterval", (ret) => clearInterval(ret as any)) - ); - - if (typeof EventTarget !== "undefined") { - hooks.push( - installHook( - EventTarget.prototype, - "addEventListener", - function (this: any, ret, args: IArguments) { - this.removeEventListener(args[0], args[1]); - } - ) - ); - } - - return { - disposeAll: () => { - hooks.forEach((h) => h.disposeAll()); - }, - }; -} diff --git a/packages/engine/src/hooks.ts b/packages/engine/src/hooks.ts new file mode 100644 index 000000000..ee8d00993 --- /dev/null +++ b/packages/engine/src/hooks.ts @@ -0,0 +1,72 @@ +const glob = typeof window === "undefined" ? global : window; + +// These will be injected in the compiled function and link to hookContext +export const overrideFunctions = [ + "setTimeout", + "setInterval", + "console", + "EventTarget", +]; + +function applyDisposer( + original: (...args: T[]) => Y, + disposes: Array<() => void>, + disposer: (ret: Y, args: T[]) => void +) { + return function newFunction(this: any): Y { + const callerArguments = arguments; + const ret = original.apply(this, callerArguments as any); // TODO: fix any? + const ctx = this; + disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); + return ret; + }; +} + +export function executeWithHooks( + modelFunction: (hookContext: any) => Promise +) { + const disposers: Array<() => void> = []; + + const executeModel = async function (this: any) { + const hookContext = { + setTimeout: applyDisposer(setTimeout, disposers, (ret) => { + clearTimeout(ret); + }), + setInterval: applyDisposer(setInterval, disposers, (ret) => { + clearInterval(ret); + }), + console: { + ...glob.console, + log: () => { + // TODO: broadcast output to console view + }, + }, + EventTarget: undefined, + }; + + if (typeof EventTarget !== "undefined") { + (hookContext.EventTarget as any) = { + ...glob.EventTarget, + prototype: { + ...glob.EventTarget.prototype, + addEventListener: applyDisposer( + EventTarget.prototype.addEventListener as any, + disposers, + function (this: any, _ret, args) { + this.removeEventListener(args[0], args[1]); + } + ), + }, + }; + } + + return modelFunction(hookContext); + }; + + return { + executeModel, + disposeHooks: () => { + disposers.forEach((d) => d()); + }, + }; +} diff --git a/packages/engine/src/modules.ts b/packages/engine/src/modules.ts index 7a0a10af7..3f1ed7fad 100644 --- a/packages/engine/src/modules.ts +++ b/packages/engine/src/modules.ts @@ -1,5 +1,6 @@ import { TypeCellContext } from "./context"; import { observable, untracked, computed, autorun } from "mobx"; +import { overrideFunctions } from "./hooks"; // import { stored } from "./storage/stored"; // import { view } from "./view"; @@ -112,5 +113,16 @@ export function getModulesFromTypeCellCode(compiledCode: string, scope: any) { "$1async function" ); // TODO: remove await? + // Adds overridden functions + // TODO: improve injection method + totalCode = totalCode.replace( + /("use strict";)/, + `"use strict"; +// Override functinos +${overrideFunctions + .map((hook: string) => `let ${hook} = this.${hook};`) + .join("\n")}\n` + ); + return getModulesFromCode(totalCode, scope); } From ead39d350e8ffb7f7fc9296d80dda0494d721d41 Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Sun, 6 Feb 2022 21:36:40 +0100 Subject: [PATCH 3/8] Add some type safety --- packages/engine/src/hooks.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/engine/src/hooks.ts b/packages/engine/src/hooks.ts index ee8d00993..b5bd671bc 100644 --- a/packages/engine/src/hooks.ts +++ b/packages/engine/src/hooks.ts @@ -1,12 +1,10 @@ -const glob = typeof window === "undefined" ? global : window; - // These will be injected in the compiled function and link to hookContext export const overrideFunctions = [ "setTimeout", "setInterval", "console", "EventTarget", -]; +] as const; function applyDisposer( original: (...args: T[]) => Y, @@ -28,7 +26,7 @@ export function executeWithHooks( const disposers: Array<() => void> = []; const executeModel = async function (this: any) { - const hookContext = { + const hookContext: { [K in typeof overrideFunctions[number]]: any } = { setTimeout: applyDisposer(setTimeout, disposers, (ret) => { clearTimeout(ret); }), @@ -36,9 +34,10 @@ export function executeWithHooks( clearInterval(ret); }), console: { - ...glob.console, - log: () => { + ...console, + log: (...args: any) => { // TODO: broadcast output to console view + console.log(...args); }, }, EventTarget: undefined, @@ -46,9 +45,9 @@ export function executeWithHooks( if (typeof EventTarget !== "undefined") { (hookContext.EventTarget as any) = { - ...glob.EventTarget, + EventTarget, prototype: { - ...glob.EventTarget.prototype, + ...EventTarget.prototype, addEventListener: applyDisposer( EventTarget.prototype.addEventListener as any, disposers, From e0d1d199a20bcba3cae97ed0a2285fcb25c7596c Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Tue, 8 Feb 2022 22:47:16 +0100 Subject: [PATCH 4/8] Add window hook --- packages/engine/src/hooks.ts | 12 ++++++++++++ packages/engine/src/modules.ts | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/hooks.ts b/packages/engine/src/hooks.ts index b5bd671bc..4a77d92a1 100644 --- a/packages/engine/src/hooks.ts +++ b/packages/engine/src/hooks.ts @@ -4,6 +4,7 @@ export const overrideFunctions = [ "setInterval", "console", "EventTarget", + "window", ] as const; function applyDisposer( @@ -41,6 +42,7 @@ export function executeWithHooks( }, }, EventTarget: undefined, + window: undefined, }; if (typeof EventTarget !== "undefined") { @@ -59,6 +61,16 @@ export function executeWithHooks( }; } + if (typeof window !== "undefined") { + hookContext.window = { + ...window, + setTimeout: hookContext.setTimeout, + setInterval: hookContext.setInterval, + console: hookContext.console, + EventTarget: hookContext.EventTarget, + }; + } + return modelFunction(hookContext); }; diff --git a/packages/engine/src/modules.ts b/packages/engine/src/modules.ts index 3f1ed7fad..39de091a1 100644 --- a/packages/engine/src/modules.ts +++ b/packages/engine/src/modules.ts @@ -118,7 +118,7 @@ export function getModulesFromTypeCellCode(compiledCode: string, scope: any) { totalCode = totalCode.replace( /("use strict";)/, `"use strict"; -// Override functinos +// Override functions ${overrideFunctions .map((hook: string) => `let ${hook} = this.${hook};`) .join("\n")}\n` From a45af95187d3adab02fe5d2aa19482f89e077bd4 Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Wed, 9 Feb 2022 13:43:34 +0100 Subject: [PATCH 5/8] Combine both hook solutions --- packages/engine/src/CellEvaluator.ts | 8 +- packages/engine/src/HookExecution.ts | 115 +++++++++++++++++++++++++++ packages/engine/src/executor.ts | 15 ++-- packages/engine/src/hooks.ts | 83 ------------------- packages/engine/src/modules.ts | 19 ++--- 5 files changed, 135 insertions(+), 105 deletions(-) create mode 100644 packages/engine/src/HookExecution.ts delete mode 100644 packages/engine/src/hooks.ts diff --git a/packages/engine/src/CellEvaluator.ts b/packages/engine/src/CellEvaluator.ts index 805b4f726..bbd101599 100644 --- a/packages/engine/src/CellEvaluator.ts +++ b/packages/engine/src/CellEvaluator.ts @@ -1,5 +1,6 @@ import { TypeCellContext } from "./context"; import { ModuleExecution, runModule } from "./executor"; +import { HookExecution } from "./HookExecution"; import { createExecutionScope, getModulesFromTypeCellCode } from "./modules"; import { isReactView } from "./reactView"; @@ -49,7 +50,11 @@ export function createCellEvaluator( onOutputChanged(error); } - const executionScope = createExecutionScope(typecellContext); + const hookExecution = new HookExecution(); + const executionScope = createExecutionScope( + typecellContext, + hookExecution.context + ); let moduleExecution: ModuleExecution | undefined; async function evaluate(compiledCode: string) { @@ -69,6 +74,7 @@ export function createCellEvaluator( modules[0], typecellContext, resolveImport, + hookExecution, beforeExecuting, onExecuted, onError, diff --git a/packages/engine/src/HookExecution.ts b/packages/engine/src/HookExecution.ts new file mode 100644 index 000000000..37c81aa6a --- /dev/null +++ b/packages/engine/src/HookExecution.ts @@ -0,0 +1,115 @@ +const glob = typeof window === "undefined" ? global : window; + +const overrideFunctions = [ + "setTimeout", + "setInterval", + "console", + "EventTarget", + "window", + "global", +] as const; + +const originalReferences: HookContext = { + setTimeout: glob.setTimeout, + setInterval: glob.setInterval, + console: glob.console, + EventTarget: glob.EventTarget, + window: window, + global: global, +}; + +export type HookContext = { [K in typeof overrideFunctions[number]]: any }; + +export class HookExecution { + public disposers: Array<() => void> = []; + public context: HookContext = { + setTimeout: this.applyDisposer(setTimeout, this.disposers, (ret) => { + clearTimeout(ret); + }), + setInterval: this.applyDisposer(setInterval, this.disposers, (ret) => { + clearInterval(ret); + }), + console: { + ...originalReferences.console, + log: (...args: any) => { + // TODO: broadcast output to console view + originalReferences.console.log(...args); + }, + }, + EventTarget: undefined, + window: undefined, + global: undefined, + }; + + constructor() { + if (typeof EventTarget !== "undefined") { + this.context.EventTarget = { + EventTarget, + prototype: { + ...EventTarget.prototype, + addEventListener: this.applyDisposer( + EventTarget.prototype.addEventListener as any, + this.disposers, + function (this: any, _ret, args) { + this.removeEventListener(args[0], args[1]); + } + ), + }, + }; + } + + if (typeof window !== "undefined") { + this.context.window = { + ...window, + setTimeout: this.context.setTimeout, + setInterval: this.context.setInterval, + console: this.context.console, + EventTarget: this.context.EventTarget, + }; + } + + if (typeof global !== "undefined") { + this.context.global = { + ...global, + setTimeout: this.context.setTimeout, + setInterval: this.context.setInterval, + console: this.context.console, + EventTarget: this.context.EventTarget, + }; + } + } + + public attachToWindow() { + overrideFunctions + .filter((name) => name !== "window" && name !== "global") + .forEach((name) => { + (glob as any)[name] = this.context[name]; + }); + } + + public detachFromWindow() { + overrideFunctions + .filter((name) => name !== "window" && name !== "global") + .forEach((name) => { + (glob as any)[name] = originalReferences[name]; + }); + } + + public dispose() { + this.disposers.forEach((d) => d()); + } + + private applyDisposer( + original: (...args: T[]) => Y, + disposes: Array<() => void>, + disposer: (ret: Y, args: T[]) => void + ) { + return function newFunction(this: any): Y { + const callerArguments = arguments; + const ret = original.apply(this, callerArguments as any); // TODO: fix any? + const ctx = this; + disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); + return ret; + }; + } +} diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 34fd91848..4da328b7c 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -1,6 +1,6 @@ import { autorun, runInAction } from "mobx"; import { TypeCellContext } from "./context"; -import { executeWithHooks } from "./hooks"; +import { HookExecution } from "./HookExecution"; import { Module } from "./modules"; import { isStored } from "./storage/stored"; import { isView } from "./view"; @@ -61,6 +61,7 @@ export async function runModule( mod: Module, context: TypeCellContext, resolveImport: (module: string) => any, + hookExecution: HookExecution, beforeExecuting: () => void, onExecuted: (exports: any) => void, onError: (error: any) => void, @@ -115,16 +116,14 @@ export async function runModule( beforeExecuting(); - const { executeModel, disposeHooks } = executeWithHooks( - (hookContext) => - mod.factoryFunction.apply(hookContext, argsToCallFunctionWith) // TODO: what happens with disposers if a rerun of this function is slow / delayed? - ); - - disposeEveryRun.push(disposeHooks); + disposeEveryRun.push(hookExecution.dispose); + hookExecution.attachToWindow(); try { - await executeModel(); + await mod.factoryFunction.apply(undefined, argsToCallFunctionWith); } finally { + hookExecution.detachFromWindow(); + if (previousVariableDisposer) { previousVariableDisposer(exports); } diff --git a/packages/engine/src/hooks.ts b/packages/engine/src/hooks.ts deleted file mode 100644 index 4a77d92a1..000000000 --- a/packages/engine/src/hooks.ts +++ /dev/null @@ -1,83 +0,0 @@ -// These will be injected in the compiled function and link to hookContext -export const overrideFunctions = [ - "setTimeout", - "setInterval", - "console", - "EventTarget", - "window", -] as const; - -function applyDisposer( - original: (...args: T[]) => Y, - disposes: Array<() => void>, - disposer: (ret: Y, args: T[]) => void -) { - return function newFunction(this: any): Y { - const callerArguments = arguments; - const ret = original.apply(this, callerArguments as any); // TODO: fix any? - const ctx = this; - disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); - return ret; - }; -} - -export function executeWithHooks( - modelFunction: (hookContext: any) => Promise -) { - const disposers: Array<() => void> = []; - - const executeModel = async function (this: any) { - const hookContext: { [K in typeof overrideFunctions[number]]: any } = { - setTimeout: applyDisposer(setTimeout, disposers, (ret) => { - clearTimeout(ret); - }), - setInterval: applyDisposer(setInterval, disposers, (ret) => { - clearInterval(ret); - }), - console: { - ...console, - log: (...args: any) => { - // TODO: broadcast output to console view - console.log(...args); - }, - }, - EventTarget: undefined, - window: undefined, - }; - - if (typeof EventTarget !== "undefined") { - (hookContext.EventTarget as any) = { - EventTarget, - prototype: { - ...EventTarget.prototype, - addEventListener: applyDisposer( - EventTarget.prototype.addEventListener as any, - disposers, - function (this: any, _ret, args) { - this.removeEventListener(args[0], args[1]); - } - ), - }, - }; - } - - if (typeof window !== "undefined") { - hookContext.window = { - ...window, - setTimeout: hookContext.setTimeout, - setInterval: hookContext.setInterval, - console: hookContext.console, - EventTarget: hookContext.EventTarget, - }; - } - - return modelFunction(hookContext); - }; - - return { - executeModel, - disposeHooks: () => { - disposers.forEach((d) => d()); - }, - }; -} diff --git a/packages/engine/src/modules.ts b/packages/engine/src/modules.ts index 39de091a1..a4d612c8e 100644 --- a/packages/engine/src/modules.ts +++ b/packages/engine/src/modules.ts @@ -1,6 +1,6 @@ import { TypeCellContext } from "./context"; import { observable, untracked, computed, autorun } from "mobx"; -import { overrideFunctions } from "./hooks"; +import { HookContext } from "./HookExecution"; // import { stored } from "./storage/stored"; // import { view } from "./view"; @@ -72,7 +72,10 @@ function createDefine(modules: Module[]) { }; } -export function createExecutionScope(context: TypeCellContext) { +export function createExecutionScope( + context: TypeCellContext, + hookContext: HookContext +) { const scope = { autorun, $: context.context, @@ -83,6 +86,7 @@ export function createExecutionScope(context: TypeCellContext) { // stored, // view, observable, + ...hookContext, }; return scope; } @@ -113,16 +117,5 @@ export function getModulesFromTypeCellCode(compiledCode: string, scope: any) { "$1async function" ); // TODO: remove await? - // Adds overridden functions - // TODO: improve injection method - totalCode = totalCode.replace( - /("use strict";)/, - `"use strict"; -// Override functions -${overrideFunctions - .map((hook: string) => `let ${hook} = this.${hook};`) - .join("\n")}\n` - ); - return getModulesFromCode(totalCode, scope); } From 5e3c9a615ff91a86011fc8dd116d32dccdcab00b Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Mon, 7 Mar 2022 18:04:09 +0100 Subject: [PATCH 6/8] Review changes --- packages/engine/src/HookExecution.ts | 96 +++++++++++----------------- packages/engine/src/executor.ts | 2 +- packages/engine/src/modules.ts | 7 +- 3 files changed, 46 insertions(+), 59 deletions(-) diff --git a/packages/engine/src/HookExecution.ts b/packages/engine/src/HookExecution.ts index 37c81aa6a..c83e83ffb 100644 --- a/packages/engine/src/HookExecution.ts +++ b/packages/engine/src/HookExecution.ts @@ -4,31 +4,44 @@ const overrideFunctions = [ "setTimeout", "setInterval", "console", - "EventTarget", - "window", - "global", + "EventTarget.prototype.addEventListener", ] as const; const originalReferences: HookContext = { setTimeout: glob.setTimeout, setInterval: glob.setInterval, console: glob.console, - EventTarget: glob.EventTarget, - window: window, - global: global, + "EventTarget.prototype.addEventListener": + glob.EventTarget.prototype.addEventListener, }; export type HookContext = { [K in typeof overrideFunctions[number]]: any }; +function setProperty(base: Object, path: string, value: any) { + const layers = path.split("."); + if (layers.length > 1) { + const toOverride = layers.pop()!; + // Returns second last path member + const layer = layers.reduce((o, i) => o[i], base as any); + layer[toOverride] = value; + } else { + (base as any)[path] = value; + } +} + export class HookExecution { public disposers: Array<() => void> = []; public context: HookContext = { - setTimeout: this.applyDisposer(setTimeout, this.disposers, (ret) => { + setTimeout: this.createHookedFunction(setTimeout, this.disposers, (ret) => { clearTimeout(ret); }), - setInterval: this.applyDisposer(setInterval, this.disposers, (ret) => { - clearInterval(ret); - }), + setInterval: this.createHookedFunction( + setInterval, + this.disposers, + (ret) => { + clearInterval(ret); + } + ), console: { ...originalReferences.console, log: (...args: any) => { @@ -36,70 +49,39 @@ export class HookExecution { originalReferences.console.log(...args); }, }, - EventTarget: undefined, - window: undefined, - global: undefined, + ["EventTarget.prototype.addEventListener"]: undefined, }; constructor() { if (typeof EventTarget !== "undefined") { - this.context.EventTarget = { - EventTarget, - prototype: { - ...EventTarget.prototype, - addEventListener: this.applyDisposer( - EventTarget.prototype.addEventListener as any, - this.disposers, - function (this: any, _ret, args) { - this.removeEventListener(args[0], args[1]); - } - ), - }, - }; - } - - if (typeof window !== "undefined") { - this.context.window = { - ...window, - setTimeout: this.context.setTimeout, - setInterval: this.context.setInterval, - console: this.context.console, - EventTarget: this.context.EventTarget, - }; - } - - if (typeof global !== "undefined") { - this.context.global = { - ...global, - setTimeout: this.context.setTimeout, - setInterval: this.context.setInterval, - console: this.context.console, - EventTarget: this.context.EventTarget, - }; + this.context["EventTarget.prototype.addEventListener"] = + this.createHookedFunction( + EventTarget.prototype.addEventListener as any, + this.disposers, + function (this: any, _ret, args) { + this.removeEventListener(args[0], args[1]); + } + ); } } public attachToWindow() { - overrideFunctions - .filter((name) => name !== "window" && name !== "global") - .forEach((name) => { - (glob as any)[name] = this.context[name]; - }); + overrideFunctions.forEach((path) => + setProperty(glob, path, this.context[path]) + ); } public detachFromWindow() { - overrideFunctions - .filter((name) => name !== "window" && name !== "global") - .forEach((name) => { - (glob as any)[name] = originalReferences[name]; - }); + overrideFunctions.forEach((path) => + setProperty(glob, path, originalReferences[path]) + ); } public dispose() { this.disposers.forEach((d) => d()); } - private applyDisposer( + private createHookedFunction( original: (...args: T[]) => Y, disposes: Array<() => void>, disposer: (ret: Y, args: T[]) => void diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 4da328b7c..6a29d517e 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -116,7 +116,7 @@ export async function runModule( beforeExecuting(); - disposeEveryRun.push(hookExecution.dispose); + disposeEveryRun.push(hookExecution.dispose.bind(hookExecution)); hookExecution.attachToWindow(); try { diff --git a/packages/engine/src/modules.ts b/packages/engine/src/modules.ts index a4d612c8e..a0d2792ca 100644 --- a/packages/engine/src/modules.ts +++ b/packages/engine/src/modules.ts @@ -1,6 +1,7 @@ import { TypeCellContext } from "./context"; import { observable, untracked, computed, autorun } from "mobx"; import { HookContext } from "./HookExecution"; +import { set as setProperty } from "lodash"; // import { stored } from "./storage/stored"; // import { view } from "./view"; @@ -86,8 +87,12 @@ export function createExecutionScope( // stored, // view, observable, - ...hookContext, }; + + Object.keys(hookContext).forEach((path) => + setProperty(scope, path, hookContext[path as keyof HookContext]) + ); + return scope; } From 9320c5f36d1c74aff39521b9a4fc3caa26997648 Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Wed, 23 Mar 2022 22:32:07 +0100 Subject: [PATCH 7/8] Review improvements --- packages/engine/src/CellEvaluator.ts | 2 +- packages/engine/src/HookExecution.ts | 46 +++++++++++++++------------- packages/engine/src/executor.ts | 11 +++++-- packages/engine/src/modules.ts | 10 ++---- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/packages/engine/src/CellEvaluator.ts b/packages/engine/src/CellEvaluator.ts index bbd101599..fb9cb9af4 100644 --- a/packages/engine/src/CellEvaluator.ts +++ b/packages/engine/src/CellEvaluator.ts @@ -53,7 +53,7 @@ export function createCellEvaluator( const hookExecution = new HookExecution(); const executionScope = createExecutionScope( typecellContext, - hookExecution.context + hookExecution.scopeHooks ); let moduleExecution: ModuleExecution | undefined; diff --git a/packages/engine/src/HookExecution.ts b/packages/engine/src/HookExecution.ts index c83e83ffb..f5363aba5 100644 --- a/packages/engine/src/HookExecution.ts +++ b/packages/engine/src/HookExecution.ts @@ -1,13 +1,15 @@ const glob = typeof window === "undefined" ? global : window; -const overrideFunctions = [ - "setTimeout", - "setInterval", - "console", +// These functions will be added to the scope of the cell +const onScopeFunctions = ["setTimeout", "setInterval", "console"] as const; + +// These functions will be attached to the window during cell execution +const onWindowFunctions = [ + ...onScopeFunctions, "EventTarget.prototype.addEventListener", ] as const; -const originalReferences: HookContext = { +export const originalReferences: ScopeHooks & WindowHooks = { setTimeout: glob.setTimeout, setInterval: glob.setInterval, console: glob.console, @@ -15,7 +17,9 @@ const originalReferences: HookContext = { glob.EventTarget.prototype.addEventListener, }; -export type HookContext = { [K in typeof overrideFunctions[number]]: any }; +export type ScopeHooks = { [K in typeof onScopeFunctions[number]]: any }; + +export type WindowHooks = { [K in typeof onWindowFunctions[number]]: any }; function setProperty(base: Object, path: string, value: any) { const layers = path.split("."); @@ -31,17 +35,13 @@ function setProperty(base: Object, path: string, value: any) { export class HookExecution { public disposers: Array<() => void> = []; - public context: HookContext = { - setTimeout: this.createHookedFunction(setTimeout, this.disposers, (ret) => { + public scopeHooks: ScopeHooks = { + setTimeout: this.createHookedFunction(setTimeout, (ret) => { clearTimeout(ret); }), - setInterval: this.createHookedFunction( - setInterval, - this.disposers, - (ret) => { - clearInterval(ret); - } - ), + setInterval: this.createHookedFunction(setInterval, (ret) => { + clearInterval(ret); + }), console: { ...originalReferences.console, log: (...args: any) => { @@ -49,15 +49,18 @@ export class HookExecution { originalReferences.console.log(...args); }, }, + }; + + public windowHooks: WindowHooks = { + ...this.scopeHooks, ["EventTarget.prototype.addEventListener"]: undefined, }; constructor() { if (typeof EventTarget !== "undefined") { - this.context["EventTarget.prototype.addEventListener"] = + this.windowHooks["EventTarget.prototype.addEventListener"] = this.createHookedFunction( EventTarget.prototype.addEventListener as any, - this.disposers, function (this: any, _ret, args) { this.removeEventListener(args[0], args[1]); } @@ -66,13 +69,13 @@ export class HookExecution { } public attachToWindow() { - overrideFunctions.forEach((path) => - setProperty(glob, path, this.context[path]) + onWindowFunctions.forEach((path) => + setProperty(glob, path, this.windowHooks[path]) ); } public detachFromWindow() { - overrideFunctions.forEach((path) => + onWindowFunctions.forEach((path) => setProperty(glob, path, originalReferences[path]) ); } @@ -83,14 +86,13 @@ export class HookExecution { private createHookedFunction( original: (...args: T[]) => Y, - disposes: Array<() => void>, disposer: (ret: Y, args: T[]) => void ) { return function newFunction(this: any): Y { const callerArguments = arguments; const ret = original.apply(this, callerArguments as any); // TODO: fix any? const ctx = this; - disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); + this.disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); return ret; }; } diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 6a29d517e..4a241b6a9 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -116,11 +116,16 @@ export async function runModule( beforeExecuting(); - disposeEveryRun.push(hookExecution.dispose.bind(hookExecution)); + disposeEveryRun.push(() => hookExecution.dispose()); hookExecution.attachToWindow(); + let executionPromise: Promise; + try { - await mod.factoryFunction.apply(undefined, argsToCallFunctionWith); + executionPromise = mod.factoryFunction.apply( + undefined, + argsToCallFunctionWith + ); } finally { hookExecution.detachFromWindow(); @@ -129,6 +134,8 @@ export async function runModule( } } + await executionPromise; + // Running the assignments to `context` in action should be a performance improvement to prevent triggering observers one-by-one wouldLoopOnAutorun = true; runInAction(() => { diff --git a/packages/engine/src/modules.ts b/packages/engine/src/modules.ts index a0d2792ca..33c87dd96 100644 --- a/packages/engine/src/modules.ts +++ b/packages/engine/src/modules.ts @@ -1,7 +1,6 @@ import { TypeCellContext } from "./context"; import { observable, untracked, computed, autorun } from "mobx"; -import { HookContext } from "./HookExecution"; -import { set as setProperty } from "lodash"; +import { ScopeHooks } from "./HookExecution"; // import { stored } from "./storage/stored"; // import { view } from "./view"; @@ -75,7 +74,7 @@ function createDefine(modules: Module[]) { export function createExecutionScope( context: TypeCellContext, - hookContext: HookContext + hookContext: ScopeHooks ) { const scope = { autorun, @@ -87,12 +86,9 @@ export function createExecutionScope( // stored, // view, observable, + ...hookContext, }; - Object.keys(hookContext).forEach((path) => - setProperty(scope, path, hookContext[path as keyof HookContext]) - ); - return scope; } From a1e26273650ee72a79d2b18ceb51f125290ddb06 Mon Sep 17 00:00:00 2001 From: Niklas Kors Date: Wed, 23 Mar 2022 22:38:55 +0100 Subject: [PATCH 8/8] Fix createHookedFunction --- packages/engine/src/HookExecution.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/engine/src/HookExecution.ts b/packages/engine/src/HookExecution.ts index f5363aba5..a182d85a9 100644 --- a/packages/engine/src/HookExecution.ts +++ b/packages/engine/src/HookExecution.ts @@ -88,11 +88,14 @@ export class HookExecution { original: (...args: T[]) => Y, disposer: (ret: Y, args: T[]) => void ) { + const self = this; return function newFunction(this: any): Y { const callerArguments = arguments; const ret = original.apply(this, callerArguments as any); // TODO: fix any? const ctx = this; - this.disposes.push(() => disposer.call(ctx, ret, callerArguments as any)); + self.disposers.push(() => + disposer.call(ctx, ret, callerArguments as any) + ); return ret; }; }