The great Blockly upgrade

[Richard Knoll (riknoll)]

It's time!

This PR upgrades us to the latest version of Blockly and removes our dependency on our fork (pxt-blockly). I went over this in the tech talk, but the basic gist of what this PR does:

1. Converts pxtblocks/ into modules
2. Moves all of the functionality that was in pxt-blockly into plugins. These are in the pxtblocks/plugins/ folder
3. Removes all of the many, many places where we were using private methods or monkey patching Blockly. There is still one monkey patch in BlockSvg, but I'm looking to get rid of it.

Here are some test links (open in incognito):

https://arcade.makecode.com/app/c5c20c036100e88b7eeca290470f8c45c41118f9-9f2b2b0e96

https://makecode.microbit.org/app/f9ad2136c00106a54df0b4686be1651471195d35-00171f0e8a

There are a few known minor bugs that were discovered in testing today. Jacqueline Russell (@Jaqster) and Sarah Rietkerk (@srietkerk) will be filing them in pxt-microbit so that they're tracked.

Known regressions (will file bugs):

1. Workspace search (ctrl+f) doesn't have the nice rendering of the live site. I need to investigate this plugin to see if I can fix it (or if I need to fork it)
2. No reindeer connectors. This will be re-enabled when Blockly 10.4 ships later this week
3. No duplicate on drag shadow blocks. The Blockly team is working on this and it will be in the next major release
4. Workspace comments have the default rendering. The Blockly team is working on this and it will be in the next major release.
5. In context translations for blocks are currently disabled. I'll re-enable these soon

Future work items:

1. Fix above regressions
2. Stop using our own custom field registry and move to Blockly's field registry
3. Move more of our fields into plugins and break the plugins into their own repo
4. Migrate our .blocks files from XML to JSON serialization

[Eric Anderson (eanders-ms)]

LGTM :). Made a small comment on package dependencies.

[Eric Anderson (eanders-ms)]

If we aren't checking in package-lock.json, we need to lock all dependencies to specific versions.

[Eric Anderson (eanders-ms)]

This is risky unless we check in package-lock.json. This introduces uncertainty around what we develop against vs. what we ship with.

[Eric Anderson (eanders-ms)]

Is now a good time to include package-lock.json?

[Richard Knoll (riknoll)]

one thing at a time! but i will revert this, not sure how it even happened

[Dmitriy Antipov (THEb0nny)]

Richard Knoll (@riknoll) tell me, for example with this code...
categoriesDiv.style.backgroundColor = (this.sourceBlock_ as Blockly.BlockSvg).getColourTertiary();

Now, do you need to specify this for Blockly.BlockSvg?
import * as Blockly from "blockly";

[Richard Knoll (riknoll)]

Dmitriy Antipov (@THEb0nny) you might be better off not taking this change... This was a pretty huge update to Blockly and I expect that all the pxt-ev3 fields will require a major rewrite.

To answer your question, though, you don't want to import blockly here as the dependency graph can get a little weird. There are two functions defined in pxt for requiring blockly and pxtblockly:

const pxtblockly = pxt.blocks.requirePxtBlockly()
const Blockly = pxt.blocks.requireBlockly();

This will import the runtime code, but not the types themselves. Some of the casts being done might throw errors

[Dmitriy Antipov (THEb0nny)]

Richard Knoll (@riknoll) I'm already doing something.
I understand from other projects that fieldeditors are not used again. For example, in microbit there is only one fieldeditors, and in arcade there are none at all. I was thinking, if I remove the field_port field, which by and large completely repeats pxtblockly.FieldImages, and in blocks using ports I set images this.fieldEditor="images". Then I will need to write in all files with blocks in which field_port was used these values this.fieldOptions.columns="4" this.fieldOptions.width="300" so that the view will be the same as before the change? Or is there some other way?

/// <reference path="../node_modules/pxt-core/localtypings/pxtblockly.d.ts"/>

const pxtblockly = pxt.blocks.requirePxtBlockly()
const Blockly = pxt.blocks.requireBlockly();

export interface FieldPortsOptions {
    columns?: string;
    width?: string;
}

export class FieldPorts extends pxtblockly.FieldImages {
    public isFieldCustom_ = true;

    constructor(text: string, options: FieldPortsOptions, validator?: Function) {
        super(text, options as any, validator);

        this.columns_ = parseInt(options.columns) || 4;
        this.width_ = parseInt(options.width) || 300;

       this.updateSize_ = (Blockly.Field as any).prototype.updateSize_;
    }
    
}

[Dmitriy Antipov (THEb0nny)]

To answer your question, though, you don't want to import blockly here as the dependency graph can get a little weird. There are two functions defined in pxt for requiring blockly and pxtblockly:

Why am I asking about import * as Blockly from "blockly"
There are field_colors there in the opt_validator?: Function constructor.

constructor(text: string, params: FieldColorEnumOptions, opt_validator?: Function) {
         super(text, params, opt_validator);

         this.paramsData = params["data"];
     }

Error in super method for opt_validator.
An argument of type "Function" cannot be assigned to a parameter of type "FieldValidator".
The "Function" type does not provide a match for the signature "(newValue: any): any".

I googled somewhere for the solution opt_validator?: Function. Found opt_validator?: Blockly.FieldValidator
But then the error is on Blockly, although I indicated above

const pxtblockly = pxt.blocks.requirePxtBlockly()
const Blockly = pxt.blocks.requireBlockly();

but when I add
import * as Blockly from "blockly";
and if I remove const Blockly, the error disappears.

Tell me how to solve this problem correctly.

I found it from here.
https://github.com/microsoft/pxt/blob/master/pxtblocks/fields/field_toggle.ts

[Richard Knoll (riknoll)]

the pxt.blocks.requireBlockly function only imports the runtime code and not type definitions like Blockly.FieldValidator. as a temporary solution, i would just declare the type as any until i figure out how to best reference those types without importing Blockly itself. if you import Blockly here, you will end up including blockly in the page more than once which could impact perf/cause weird things to happen.

and yes, wherever possible you should just use the fields that are defined in pxt (such as FieldImages) and not pxt-ev3. it might be worth looking into if there is a way to deprecate all of them; this will likely change how the blocks look/function but will certainly make keeping up with latest pxt less of a headache.

[Dmitriy Antipov (THEb0nny)]

Richard Knoll (@riknoll) (this.sourceBlock_ as Blockly.BlockSvg) can it be replaced this.sourceBlock_ as any?

[Richard Knoll (riknoll)]

Dmitriy Antipov (@THEb0nny) i'm afraid not, there were major changes made to Blockly in the version we updated. many of the methods/properties we were using before either no longer exist or were made private (and thus inaccessible). if you are getting an error for something on Blockly.BlockSvg, I think it's likely something that was removed in that way and you need to either figure out the new way to access those properties or re-implement it from scratch.

this is what i meant when i said that all of the field editors will require a major rewrite (if you want to use version 10.X.X of pxt).

alternatively, you can take a dependency on the v9 branch of pxt which uses the old version of Blockly.

[Dmitriy Antipov (THEb0nny)]

Richard Knoll (@riknoll) well, I made almost all the changes.
microsoft/pxt-ev3#1052

I had to import BlockSvg } this way.

import { BlockSvg } from "blockly";

The only problem left is with field_color. The choice of value does not work there, but there is no error.
For some reason the className attribute is not set.

There is something wrong with the colornumber field with the class setting. Because the class is not installed.

//% blockId=colorEnumPicker block="%color" shim=TD_ID
    //% weight=0 blockHidden=1
    //% color.fieldEditor="colornumber"
    //% color.fieldOptions.colours='["#f12a21", "#ffd01b", "#006db3", "#00934b", "#000000", "#6c2d00", "#ffffff", "#dfe6e9"]'
    //% color.fieldOptions.columns=3
    //% color.fieldOptions.className='legoColorPicker'
    export function __colorEnumPicker(color: ColorSensorColor): number {
        return color;
    }

[Dmitriy Antipov (THEb0nny)]

Richard Knoll (@riknoll) In your field_colors showEditor_() was removed, where the class was installed. Was this done intentionally?

showEditor_() {
            super.showEditor_();
            if (this.className_ && this.picker_)
                pxt.BrowserUtils.addClass(this.picker_ as HTMLElement, this.className_);
        }

[Dmitriy Antipov (THEb0nny)]

I fixed all the fields pxt-ev3.