refactor: convert registry files to es6 classes #5940
Conversation
core/registry.js
Outdated
| @@ -73,28 +73,31 @@ exports.DEFAULT = DEFAULT; | |||
|
|
|||
| /** | |||
| * A name with the type of the element stored in the generic. | |||
| * @param {string} name The name of the registry type. | |||
| * @constructor | |||
| * @template T | |||
| * @alias Blockly.registry.Type | |||
There was a problem hiding this comment.
For the other classes you've moved the @alias down to the constructor as well. Not sure which one makes the JsDoc tool happier but I had been assuming it was on the constructor.
There was a problem hiding this comment.
Done, for consistency. I'm not sure and slightly expect I'll have to do a cleanup pass when I get to API extractor; so far I've been ignoring the JsDoc tool.
core/contextmenu_registry.js
Outdated
|
|
||
| exports.contextMenuRegistry = ContextMenuRegistry.registry; |
There was a problem hiding this comment.
Why are we exporting this (and for shortcut registry) now? Is it just as a shorthand for ContextMenuRegistry.registry? I don't like introducing API/naming changes (also needs documentation changes if we intend developers to use this) buried in what seems like a refactoring change.
I think this is a good idea, it would match the fieldRegistry and class registry patterns, but also needs a few other updates like changing the internal usages and the jsdoc for the class explaining how to use the registry.
There was a problem hiding this comment.
Good point. I've backed it out and filed #5950 to track and/or discuss.
The basics
The details
Resolves
Part of #5860
Proposed Changes
registryvariables that are exported under appropriate names.resetfunctions to the shortcut registry and context menu registry.Typeobject inregistry.jsto an es6 class.Behavior Before Change
Clearing the shortcut registry or context menu registry in tests was done by calling the constructor.
Behavior After Change
Now clearing those registries is done by calling
reset.Reason for Changes
Movement toward Typescript.
Test Coverage
Documentation
Additional Information
Typescript warns that
keyMap_andregistry_and so on can be undefined, because I'm setting them in theresetfunction instead of directly in the constructor. Typescript has the definite assignment assertion operator to deal with this.