Convert blocks/ and generators/ to TypeScript
#6828
Comments
|
I took a quick crack at converting some of the generators files to see what broke. What I found:
Adding properties could be done with mixins, I think, but I'm not sure if that helps. As a test I tried something similar to what we do for blocks: The declaration of
Later, the place where I set |
|
Hey there, I migrated a big chunk of I used abstract and anonymous classes to model the different objects and the internal state It will need review since I miss most of the context |
|
Hi @IlCallo! As it happens I actually have a 90% complete PR for Do post a draft pull request if you like; even if not complete it would be interesting to look at. |
|
I ended up adding a lot of If you already completed most of it, I'll wait for you and add my comments/suggestions when possible, since my version is slightly customized compared to the original on in the codebase |
|
Would it be possible for Blockly to accept classes instead of plain object literals? You said you have a pending PR / branch for procedures migration, can you link it to me? |
|
I actually managed to get a typesafe implementation which also works (or seems to work) by using an IIFE and many Classes support would still be better IMO, but here's an example Blocks["my_procedure_def"] = (() => {
/* procedure internal state, accessed without this since is within scope */
let statementConnection_: Connection | undefined | null;
let hasStatements_: boolean | undefined;
let arguments_: string[] = [];
let paramIds_: string[] | undefined;
let argumentVarModels_: VariableModel[] = [];
/**
* Add or remove the statement block from this function definition.
* @param hasStatements True if a statement block is needed.
*/
function setStatements_(this: BlockSvg, hasStatements: boolean) { // <==== NOTICE WE ARE MANUALLY TYPING THIS
if (hasStatements_ === hasStatements) { // <==== INTERNAL STATE IS ACCESSIBLE WITHOUT "this.", THIS WORKS WITH OTHER METHODS TOO
return;
}
if (hasStatements) {
this.appendStatementInput("STACK").appendField(
Msg["PROCEDURES_DEFNORETURN_DO"]
);
if (this.getInput("RETURN")) {
this.moveInputBefore("STACK", "RETURN");
}
} else {
this.removeInput("STACK", true); // <==== "this" REFERENCES ARE RELATED TO "BlockSvg" CONTEXT
}
hasStatements_ = hasStatements;
}
// ... OTHER METHODS
/**
* Block for calling a procedure with no return value.
*/
function init(this: BlockSvg) {
// ...
}
return {
setStatements_,
// ... other methods which need to be publicly accessible
init,
}
})();Unluckily you'd then need to call all internals methods using |
|
Small update: apparently wrapping the scope using functions and effectively "hiding" them from outside created some issue for which I wasn't able to pinpoint the root cause Removing those declaration statements and updating the interface DefInternalState {
arguments_: string[];
argumentVarModels_: VariableModel[];
statementConnection_: Connection | undefined | null;
hasStatements_: boolean | undefined;
paramIds_: string[] | undefined;
}
type DefBlockSvgWithInternalState = BlockSvg & DefInternalState;
Blocks["my_procedure_def"] = (() => {
/**
* Add or remove the statement block from this function definition.
* @param hasStatements True if a statement block is needed.
*/
function setStatements_(this: DefBlockSvgWithInternalState, hasStatements: boolean) { // <==== NOTICE WE ARE MANUALLY TYPING "THIS"
if (this.hasStatements_ === hasStatements) {
return;
}
if (hasStatements) {
this.appendStatementInput("STACK").appendField(
Msg["PROCEDURES_DEFNORETURN_DO"]
);
if (this.getInput("RETURN")) {
this.moveInputBefore("STACK", "RETURN");
}
} else {
this.removeInput("STACK", true); // <==== "this" REFERENCES ARE RELATED TO "BlockSvg" CONTEXT
}
this.hasStatements_ = hasStatements;
}
// ... OTHER METHODS
/**
* Block for calling a procedure with no return value.
*/
function init(this: DefBlockSvgWithInternalState) {
// ...
this.arguments_ = [];
this.argumentVarModels_ = [];
setStatements_.call(this, true);
this.statementConnection_ = null;
}
return {
setStatements_,
// ... other methods which need to be publicly accessible
init,
}
})(); |
|
Closing this: the remaining OPTIONAL item has been added to #6920, and the other two items are… already items of their own. There is room to improve things but migration proper is complete even if the resulting code can certainly be improved! |
This is a tracking bug for the remaining work to migrate the Blockly codebase to TypeScript, following the successful migration of
core/.This is part of #5857, and will resolve #6248 and probably also #2995.
Work to be done
scripts/gulpfiles/build_tasks.jsto feed the contents ofblocks/andgenerators/throughtsc.tsconblocks/andgenerators/#6836tests/bootstrap.js, playgrounds, etc. to load blocks and generators frombuild/src/when loading uncompressed.goog.requireso don't care about file location.imports)..js->.tsmigration #6837BlockDefinition(incore/blocks.ts) to make defining blocks easier and preferably less error prone.BlockDefinitionisAnyDuringMigration; this works but means thattscprovides no assistance in verifying block definitions—for example, it does not check to make sure thatBlockmethods are being called (or overridden) correctly.BlockDefinitionas{[Property in keyof Block]?: Block[Property]} & {[key: string]: unknown}, but this is not perfect: e.g. mixin methods do not get the correct type forthis.blocks/, to verify that the above works as intended.blocks/math.jsto TypeScript #6900.blocks/blocks/blocks.jsto TypeScript #7193)blocks/colour.jsto TypeScript #6901)blocks/lists.jsto TypeScript #6902)blocks/logic.jsto TypeScript #7003)blocks/loops.jsto TypeScript #6957)blocks/math.jsto TypeScript #6900)blocks/procedures.jsto TypeScript #7192 )blocks/text.jsto TypeScript #6958)blocks/variables.jsandblocks/variables_dynamic.jsto TypeScript #7001)blocks/variables.jsandblocks/variables_dynamic.jsto TypeScript #7001)generators/Provide more user-friendly generator APIs #7326
Make
CodeGeneratorabstract #7401dart.js
javascript.js
lua.js
php.js
python.js
The text was updated successfully, but these errors were encountered: