Description
openedon Aug 6, 2024
Invalid program state
One of Blockly's principles is that any connected group of blocks is a valid program. Not necessarily a correct program, but one which could compiled to syntactically valid code in a supported language and then run. This is leveraged by a number of Blockly projects, which generate and run code on every workspace update.
In the process of investigating #7950, it came to light that when a procedures_ifreturn
block is inserted into a procedures_defnoreturn
this principle is momentarily violated, because the ifreturn
block's VALUE
input is only removed afterwards, by its onchange
handler. A quick investigation of other onchange
handlers suggested that many of them also contained code smells for invalid program state.
In addition to potential code-generation-correctness issues, the invalid state prevents undo from working correctly—in this case because undo attempts to reattach a return value to the ifreturn
block before it removes the ifreturn
block from the defnoreturn
block.
Proposal 1: Reaffirm that Blockly does not permit invalid program state
We should reaffirm the principle that Blockly does not allow invalid program states, and clarify that this means even momentarily:
- It should not be possible to connect blocks if doing so would result in an invalid program.
- If there are situations where it is not reasonably practical to prevent a program from being temporarily invalid (perhaps due to changes-at-a-distance, e.g. modifying procedure definitions), Blockly should ensure that the program is made valid again no later than the end of the current microtask (and ideally before the method call that initiated the change returns)—before any events are fired that might cause theinvalid state to be observed.
- The program can be made valid by disabling, disconnecting or deleting blocks as needed (but ideally with due regard to positive user experience).
Proposal 2: Provide a synchronous mechanism for ensuring program validity invariants are maintained
This would replace the use of onchange
handlers with some other, more specific and synchronous mechanism, akin to field validators. Ideally this mechanism should be:
- Limited in scope, so that only "affected" blocks are validated/notified of changes.
- Or, where a wide scope is needed, efficient, so that the validator is only called once per change, not once per change * block.
- Only be called for changes that affect the program structure (i.e. connection/disconnection and possibly creation/mutation/deletion, but not click/drag/viewport change etc.).
- Able to:
- Allow validator to reject a proposed connection (preventing shadow from appearing).
- This could subsume the existing type-checking system.
- Allow affected block(s) to update themselves (and other blocks) when a connection does occur, and later undo those changes when disconnected.
- Allow validator to reject a proposed connection (preventing shadow from appearing).
- Flexible enough to handle at least all the validation checks/fixes performed by existing
onchange
handlers.
(For LambdaMOO programmers: the check vs. update distinction is akin to :acceptable
vs. :accept
in LambdaCore: either can allow or reject a move, but only the latter should have any side effects.)
In the case of the procedures_ifreturn
block, this would mean that the ifreturn
block could remove its VALUE
(return value) input (and thereby detach any child blocks from it) before its pending connected to a procedures_defnoreturn
block is effected.
Scope
While it is desirable that this machinery call the minimum number of validator functions when considering / making a connection, it is not sufficient for only the immediately-involved blocks to be validated.
- Calling a validator on the two blocks being directly connected lets one implement type checking, or have binary operator blocks update the types of their other inputs/outputs.
- Calling a validator on all the blocks in the subtree being attached/detached lets blocks like
controls_flow_statements
(break/continue) enable or disable themselves depending on whether they are enclosed in a loop block or not, orprocedures_ifreturn
to add/remove itsVALUE
input. - There may be use-cases for blocks to be able to "validate" (or at least synchronously listen to) all changes, as the
onchange
handler currently allows but in a more timely way—or even all proposed changes.
Proposal 3: Deprecate block onchange
methods
We can and probably should continue to support the onchange
mechanism indefinitely for backwards compatibility with developer's custom blocks, we should stop using onchange
methods in the library blocks and we should discourage their use by developers.
- Almost all the existing
onchange
methods are used to enforce program validity invariants (see survey in separate comment below), but they can only ever do so belatedly. It seems very likely that anyonchange
methods not used to enforce program validity could also be converted to use a proposed synchronous validation mechanism instead (just as we currently use field validators for block mutation). - Block
onchange
handlers are very powerful, because they can react to any change on the workspace, but also very inefficient, because a separate change listener is registered for every such block on the workspace; this means that a change listener is called for every such block on the workspace for every change, regardless of whether the change affects the block in question, and a workspace with N copies of a particular block will call the block definition'sonchange
method N times for every event, with onlythis
differing between calls. If fully-general event listening is useful will in many cases be better for developers to register a single change listener at the workspace level that will handle updates for all relevant blocks.