[Google Blockly] strip user code

[mikeharv]

In older Blockly labs (Maze, Artist, Play Lab), there are some differences between the raw generated code that we feed into the JS interpreter and that which we show the student. For example, in these labs, additional code is used to prevent us from attempting to execute infinite loops. The generated code can also include block ids and lab-specific namespaces. Here is an example program on CDO Blockly and the associated "Show Code" modal:

Here is the same program on Google Blockly:

Note that the more complicated-looking code is still what we actually execute. CDO Blockly was accomplishing this cleanup with a call to a custom strip function which removes or modifies some stuff through regular expressions. We can actually use the same strip function to process code that Google Blockly generates, so I moved it (and some related constants) into our utils file. Some of the constants were actually already defined in both wrappers which felt a bit redundant.

The only functional change relates to differences in how block ids are generated.

Previously, it was sufficient to remove block ids by looking for a series of numbers:

.replace(/(,\s*)?'block_id_\d+'\)/g, ')')

The 'block_id_\d+' part above matches the literal string 'block_id_' followed by \d+, which matches one or more digits. To make it match any characters (except quotes), I replaced the latter with [^']+.

With the changes in this branch, the code we show matches the CDO Blockly baseline. (Nit: there's a slight indentation difference inside the loop, but I think that's fine.)

Warning!!

The AP CSP Create Performance Task is in progress. The most critical dates are from April 3 - April 30, 2024. Please consider any risk introduced by this PR that could affect our students taking AP CSP. Code.org students taking AP CSP primarily use App Lab for their Create Task, however a small percent use Game Lab. Carefully consider whether your change has any risk of alterering, changing, or breaking anything in these two labs. Even small changes, such as a different button color, are considered significant during this time period. Reach out to the Student Learning team or Curriculum team for more details.

Links

Testing story

In addition to testing the indented behavior with the change, I also made sure I was still able to execute my code in this Maze level (with both versions of Blockly), as well as a couple random projects of other labs. No regressions found. The changes to the code is only for some very specific matched expressions, so this doesn't feel overly risky.

Making this change also required reverting a migration-related Minecraft change: 3c8902b

In order to get Minecraft migrated, I switched from using Generator.blocksToCode to getWorkspaceCode(). This change worked because there was no difference in the two, and was "needed" because blocksToCode wasn't defined yet. However, the generated code here is passed to the interpreter, so it needs to be the raw, un-stripped code. If we remove the infinite loop logic from the generated code given to the interpreter, then it's easy for students to get into frozen state (see pics). For this reason, it makes sense to go back to using the generator function, now that it has been defined.

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

• Tests provide adequate coverage
• Privacy and Security impacts have been assessed
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[molly-moen]

would it be worthwhile to move this code into a wrapper function so we don't have to repeat it? Does the reference to studioApp().initializationBlocks mean we can't move this into the wrapper?

[mikeharv]

Potentially... although the changes in these craft files are really just a straight revert of 3c8902b

There's quite a few variations of the logic here in the various lab files, so I think it would take some thought to figure out exactly what we want to abstract and move to the wrapper.

[molly-moen]

👍 makes sense