Pass block info to extension methods #2102
Conversation
Sometimes a single extension method needs to service several different instances of the same opcode. This can happen with variables or custom procedures, for example. This change allows the extension method to inspect the `blockInfo` for instance data, including arbitrary extension-specific properties if necessary.
| } | ||
|
|
||
| blockInfo.func = blockInfo.func ? this._sanitizeID(blockInfo.func) : blockInfo.opcode; | ||
| if (blockInfo.opcode) { |
There was a problem hiding this comment.
Any reason for rearranging this conditional (from the previous structure of falsey check and early exit)?
There was a problem hiding this comment.
It's kind of silly... but this isn't legal JavaScript:
default:
if (!blockInfo.opcode) {
throw new Error('Missing opcode for block');
}
const funcName = "stuff"; // it's not legal to make a new `const` hereYou can do it if you introduce a new scope with {} which sort of naturally led me to the arrangement proposed here...
There was a problem hiding this comment.
Hmmmm okay. I think I'd prefer adding the new block of scope only because it's easier to read an early exit vs. keeping track of which block of code corresponds to which part of the if/else etc. Esp. since there are nested ifs involved here.
| const serviceObject = dispatch.services[serviceName]; | ||
| if (!serviceObject[funcName]) { | ||
| // The function might show up later as a dynamic property of the service object | ||
| log.warn(`Could not find extension block function called ${funcName}`); |
There was a problem hiding this comment.
Should this function exit after warning here? Below, the function RHS of the arrow function is just going to get bound to undefined(args, util, blockInfo) right?
There was a problem hiding this comment.
Since it's looking up serviceObject[funcName] in the body of the (anonymous) function, it's possible that serviceObject[funcName] will become a valid function call between now and the first time it gets called. This could happen if the extension dynamically generates methods and monkey-patches itself, for example, especially if it needs to download some data first.
I don't expect this to be common but technically the way we make "remote" service calls doesn't even do this check at all, so I figured that if it's allowed there it should be allowed here. 🤷♂️
cwillisf
force-pushed
the
pass-blockInfo-to-extension-methods
branch
from
228a2c3 to
ab84dd0
Compare
Resolves
This is a supporting change for the extensionification project.
Proposed Changes
Extension methods now receive a third argument which contains the section of the
getInforesults which the extension provided for the block being executed.Reason for Changes
Sometimes a single extension method needs to service several different instances of the same opcode. This can happen with variables or custom procedures, for example. This change allows the extension method to inspect the
blockInfofor instance data, including arbitrary extension-specific properties if necessary.Test Coverage
There is now a check check for correct
blockInfobeing passed into an extension method intest/integration/internal-extension.js.