-
Notifications
You must be signed in to change notification settings - Fork 35
Refactor Hooks #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Refactor Hooks #295
Changes from 3 commits
53bcd3d
9f550e7
ead39d3
e0d1d19
c77a207
e49c049
a45af95
5e82fbf
5e3c9a6
b7d58d5
9320c5f
fe22a2e
a1e2627
f6b944b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
// These will be injected in the compiled function and link to hookContext | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that your solution doesn't work if I'd call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Solved if you also inject
|
||
export const overrideFunctions = [ | ||
"setTimeout", | ||
"setInterval", | ||
"console", | ||
"EventTarget", | ||
] as const; | ||
|
||
function applyDisposer<T, Y>( | ||
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<T>( | ||
modelFunction: (hookContext: any) => Promise<T> | ||
) { | ||
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, | ||
}; | ||
|
||
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]); | ||
} | ||
), | ||
}, | ||
}; | ||
} | ||
|
||
return modelFunction(hookContext); | ||
}; | ||
|
||
return { | ||
executeModel, | ||
disposeHooks: () => { | ||
disposers.forEach((d) => d()); | ||
}, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't this logic the same as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code inside scope will be inserted outside of the So this doesn't work:
But this does:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure? I doubt this tbh (that option 1 doesn't work) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Think I know why. It's because I didn't feed the hookContext into scope. Will fix :) |
||
.map((hook: string) => `let ${hook} = this.${hook};`) | ||
.join("\n")}\n` | ||
); | ||
|
||
return getModulesFromCode(totalCode, scope); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't remember why I changed it. I can revert it back if you prefer the const