Update to Blockly 11 #67
Conversation
|
Just for my understanding. Is there any reason why we are not directly updating to Blockly V12? |
|
@TannerGabriel we want to publish versions for both rather than leapfrog but I’m going to try to upgrade this to 12 shortly. Are you using this plugin outside of App Inventor? |
|
@TannerGabriel fyi, it seems Blockly 12 upgrades cleanly #68 |
mjgallag
force-pushed
the
update-to-blockly-11
branch
from
d4e6bfc to
92ad383
Compare
|
@mjgallag Thanks for testing the upgrade to Blockly V12. I'm currently building an editor for embedded systems, which requires both static types and scoped variables. The lexical variables plugin seems to be the best starting point for this. |
#68 (comment) RaspberryPiFoundation/blockly#7777 RaspberryPiFoundation/blockly#6786 https://developers.google.com/blockly/guides/contribute/core-architecture/render-management Co-Authored-By: TannerGabriel <40315960+TannerGabriel@users.noreply.github.com>
|
@mark-friedman @TannerGabriel when reviewing #76 I realized this was a bigger issue so I've addressed it in this pull with 64af1e9. Essentially we can now delegate to Blockly's new/improved init/render ... when we don't it can break things. I'd appreciate a quick test/review from both of you if you've got the time, no rush. |
I'll try and give it a test sometime this weekend. |
|
@mark-friedman thanks, up to you, it can wait til next week. |
|
@mark-friedman I just merged @TannerGabriel's fix for the block warning icon so I believe this pull currently addresses all known issues with Blockly 11. I don't have a strong opinion, but perhaps for simplicity we leave it unmerged until App Inventor testing is further along with the plan being you can upload it to npm as |
|
@mjgallag I just retested, and the two original bugs are gone. Everything works fine except for one minor issue with functions that have call blocks: When deleting a function with an associated call block, the call block’s inputs (parameters) don’t update immediately. If you re-add a function with the same name, the parameters only refresh after first opening the mutation (see video). In contrast, the current master version clears the function immediately and removes all inputs. Screencast.from.2025-08-31.16-09-40.webmI didn't notice anything else while building some small example projects. |
There is a whole bunch of code that was removed in commit 64af1e9 for reasons that I don't really understand. Much of it is in blocks/procedures.js, which might explain the issue we're seeing with procedure blocks. |
|
@TannerGabriel thanks for the thorough testing! I will take a look at this one but feel free to submit a pull if you figure it out. @mark-friedman I just tested with 64af1e9 reverted and this bug is still there so it's related to Blockly 11, not my changes ... which is good because I was worried my understanding might be incorrect when I saw this bug. Basically in older versions of Blockly rendering was expensive because there was no queue, so one would pause the rendering, make the changes, then manually call render at the end. Now that there is a queue that is unnecessary and so long as you use the Blockly API methods they queue the renders for you. Init is also called for you so long as you use the API methods, it seems this wasn't always the case in the past ... at least this is my current understanding. So I figured I'd remove the very similar code in procedures that @TannerGabriel and I removed in lexical-variables.js that was the cause of the timing issues. |
|
Ah, I see. The code that was removed was all setting up for, and then doing, the re-rendering. Makes sense. |
|
@mark-friedman @TannerGabriel just fixed the bug, was an easy one ... 0e66f92. |
|
@mark-friedman the other small change in the rendering/init fix was rather then removing the body input and then re-adding it, which as @TannerGabriel found, causes you to have to redo a bunch of stuff, I just move it to the end, see 64af1e9#diff-393d0b1241e431a943baabb5eb59a3b6fafd0819a61d15ceb44a865dd40d8bdaR326 & 64af1e9#diff-23366c062bd5dfb7f10bf8a521ee0f76f0ad5aa5b4be8a5e84ee990c781afc96R231 which queue's a rerender. |
|
There's another small "timing" bug related to procedure deletion.
Note that the call block won't display the procedure name until you click again somewhere in the workspace or toolbox. The old version of the plugin does not have this bug. |
|
@mark-friedman thanks for testing, will take a look now. |
setWarningText checks for warning icon https://github.com/google/blockly/blob/blockly-v11.2.2/core/block_svg.ts#L972
Blocky 10 caused `this.setValue(text)` to stack overflow so it was replaced with `this.doValueUpdate_(text)` which stops render & code generation. It can be removed as it's inside the field validator so the value will be set after successful validation. ewpatton/appinventor-sources@bd2cec9 ewpatton/appinventor-sources@6c74e5b#diff-179ada7bc9ef841c7b52e24fe50097e0ee0aad37f7f1e4d19fec4f5323ef9785R41 https://developers.google.com/blockly/guides/create-custom-blocks/fields/validators
|
@mark-friedman it's fixed. I followed the commit history back to original code in app inventor from 12 years ago and I can't figure out why it was ever set mit-cml/appinventor-sources@044b60d#diff-179ada7bc9ef841c7b52e24fe50097e0ee0aad37f7f1e4d19fec4f5323ef9785R11. Either it was never needed or it was working around a bug ... that hopefully has been fixed so that I didn't regress anything. |
|
Just a note that Evan had a suggestion to make this version 11.0.0 when we publish it and in general to make the major version number match the Blockly version that it depends on. That sounds like a good idea to me. |
No description provided.