refactor(blocks): Migrate blocks/lists.js to TypeScript
#6902
Conversation
Not running clang-format on this as for some reason it decides half way through the file to indent everything after that by an extra four spaces (and then has to re-wrap a bunch of lines that are consequently too long).
It turns out that the way I originally specified the types for the mixins meant that they were all 'any', which is a bit useless. Change them so that tsc actually typechecks properties (including method calls) on the mixed-in blocks, and then fix the numerous additional type errors which doing this revealed. (By "fix", I mean apply "as" casts and "!"s as required to suppress type errors from tsc. Actually fixing the code in a way that makes the blocks meaningfully more bulletproof is left as an exercise to the reader - sorry, I mean: will be dealt with in a future PR.)
| /** Type of a 'lists_create_with' block. */ | ||
| type CreateWithBlock = Block&ListCreateWithMixin; | ||
| interface ListCreateWithMixin extends ListCreateWithMixinType { | ||
| itemCount_: number; |
There was a problem hiding this comment.
This is because this is added in the init function? Could we just define it in the mixin instead of defining it in the constructor?
Here & below.
There was a problem hiding this comment.
I was wondering about that. I admit I am not 100% sure about the mechanics of block creation, so I thought it was safer to leave this as-is for now, but I'll add a note to #6920 to consider that.
Just to confirm: do you believe it is safe to init properties via (primitive-valued) properties on the mixin, instead of in the init method?
There was a problem hiding this comment.
I do believe that that is safe. We actually do that in the procedure blocks (eg). All of the values get mixed into the block before the init method is called. So it's safe!
The basics
npm run formatandnpm run lintThe details
Resolves
Part of #6828.