blockly-samples
blockly-samples copied to clipboard
chore: convert field-slider plugin to TypeScript
Description
- Updated
plugins/field-sliderto use TypeScript for its src code and demo test code. - Updated
plugins/dev-toolsto require Blockly > 8.0.3 to be compatible.
Note
- The
blockly.d.tsexport in blockly core is not compatible with this package prior to8.0.3. - The
test/*.mocha.jsfiles are left unchanged since the dev scripts do not support TS currently. - The
.d.tsfiles forplugins/field-sliderwill not be generated due to an issue withplugins/dev-scripts. A fix was found for that issue, though it won't be accepted until the 'core Blockly to TS' is resolved since there is some crossover in the fix's implementation.
Discussion
- There are some
TODOnotes related to type information for core Blockly packages, though there's a large TS conversion in that incoming package which will likely impact the type information. Should:- Individual issues be created for each TODO?
- One group issue be created for all of those TODOs?
- Those TODOs be attributed to the incoming TS conversion?
- The TODOs be removed for now?
I'm assigning @maribethb as well, because this needs her opinions on bumping the dev-tools dependency. Bumping the min version of Blockly requires a major bump for the dev-tools version, and I just want to double check we're cool with doing that.
The build issue is an interesting one. When running a clean install from plugins/field-slider, the build works fine. When running a clean install from the root, though, there's an issue with the blockly package for building. (The resulting plugins/field-slider/package-lock.json also varies based on where the install is from.) My guess is it's related to how lerna is linking the packages, though I'll keep investigating.
Can you insert a commit that changes the file extensions
.jsto.tswithout actually modifying the contents of the files? That way we can ensure that git understands we're just renaming files, not deleting them and creating new ones, which is important for maintaining ourgit blame.
Done!
This will need the change in #1299 as well.
Looks like blockly-scripts doesn't work with the override keyword. I removed it from render_ and showEditor_, though I'd originally added it to those since an override comment was present.
In TypeScript, override ensures that the method we're trying to override exists in the parent. This is pretty handy if, for example, the render_() method were updated to renderInternal() or something similar since the compiler would catch it. I'm not sure if this is a blocking issue, though. (See the TypeScript 4.3 documentation for more about override.)
It's also probably worth investigating why blockly-scripts doesn't work with override.
For the failing tests, it looks like the updated Blockly imports break outside of my changes.
For example, going to blockly-samples/plugins/block-plus-minus/test/if.mocha.js, Blockly.Dart does not exist.