Add pxtservices package and iframe driver #9912
Conversation
| addEventListener(event: "workspacesync", handler: (ev: pxt.editor.EditorWorkspaceSyncRequest) => void): void; | ||
| addEventListener(event: "workspaceloaded", handler: (ev: pxt.editor.EditorWorkspaceSyncRequest) => void): void; | ||
| addEventListener(event: "workspacediagnostics", handler: (ev: pxt.editor.EditorWorkspaceDiagnostics) => void): void; | ||
| addEventListener(event: "editorcontentloaded", handler: (ev: pxt.editor.EditorContentLoadedRequest) => void): void; |
There was a problem hiding this comment.
duplicate declaration of "editorcontentloaded" handler.
| return new Promise((resolve, reject) => { | ||
| message.response = true; | ||
| message.id = this.nextId++ + ""; | ||
| this.pendingMessages[message.id] = { |
There was a problem hiding this comment.
Noting a potential leak: All sent messages are wrapped in a promise and added to pendingMessages, which resolve once a reply is received. Is a reply received for every message sent?
There was a problem hiding this comment.
only requests are wrapped in a promise. all requests with message.response set to true should indeed receive a response from the editor. i guess we could time out requests, but i think it's a minor scenario and i worry it might affect some long running requests (like starting a tutorial on a low end chromebook)
i guess i could implement some sort of heartbeat mechanism where the iframe reports back what messages are still active, but that might be overkill... thoughts?
There was a problem hiding this comment.
Ok, that seems fine then. If we expect every message sent this way to receive a response, then promises won't leak except when something else is already broken. Adding a heartbeat seems like overkill :)
| if (!data || !/^pxt(host|editor|pkgext|sim)$/.test(data.type)) return; | ||
|
|
||
| if (data.type === "pxteditor") { | ||
| if (data.id && this.pendingMessages[data.id]) { |
There was a problem hiding this comment.
Noting a potential leak: The promise associated with sent messages that never receive a their expected reply could hang forever. This driver may want to run a pending message checker in the background (on a timeout), to reject and clear pending messages that haven't received a reply in a certain amount of time.
This PR adds a new top-level package called pxtservices to be shared between our many webapps. Currently it just contains a single file that defines a nifty class for driving iframes that embed the makecode editor. Features include:
addEventListener()(also type checked)As part of this, I also fixed three editor events that were being incorrectly fired with the "pxteditor" type instead of "pxthost". The "pxteditor" versions still fire so as not to break existing implementations.
I also updated the makecodeEditorService to use this.