-
Notifications
You must be signed in to change notification settings - Fork 67
Facilitate running custom javascript on the main JS process #63
base: master
Are you sure you want to change the base?
Conversation
@@ -9,6 +9,8 @@ module.exports = { | |||
appCmdQuit: "app.cmd.quit", | |||
appEventReady: "app.event.ready", | |||
appEventSecondInstance: "app.event.second.instance", | |||
appExecuteJavaScript: "app.execute.javascript", |
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.
Could you rename that to appCmdExecuteJavascript: "app.cmd.execute.javascript"
?
@@ -9,6 +9,8 @@ module.exports = { | |||
appCmdQuit: "app.cmd.quit", | |||
appEventReady: "app.event.ready", | |||
appEventSecondInstance: "app.event.second.instance", | |||
appExecuteJavaScript: "app.execute.javascript", | |||
appExecuteJavaScriptCallback: "app.execute.javascript.callback", |
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.
Could you rename that to appEventExecutedJavascript: "app.event.executed.javascript"
?
}; | ||
try { | ||
client.write(json.targetID, consts.eventNames.appExecuteJavaScriptCallback, { | ||
reply: await evalAsync(json.code, json) |
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.
Could you rename reply
to success
?
(async function () { | ||
let evalAsync = (ev, json) => { | ||
return new Promise((resolve, reject) => { | ||
eval(ev); |
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.
eval
code will have access to resolve and reject here ?
I'm preparing another PR for go-astilectron that interfaces with this.
I figured this works well with astilectron's approach of forward compatibility. ie. electron can release new versions but astilectron doesn't necessarily need an update to support them. Unfortunately, this can leave you stranded if you want to user features in the new electron version.
Obviously this has all the established security caveats of using eval. That's a consideration for the developer who utilizes the code though, IMO.
Note that the code is written so that you have to call
resolve()
andreject()
with the result. This is to facilitate asynchronous JavaScript code.Very new to this codebase so please feel free to point out better ways of addressing this.