-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Description
This is a tracking bug for the work the Blockly team is doing in Q3 2021 to transition the codebase from goog.provide to goog.module, as recently announced.
This bug will track any outstanding tasks not covered by other more specific bugs, provide documentation for how the migration will be done and so on.
If you have questions about why we are doing this work and what the consequences will be for developers who use Blockly please ask them in the Blockly forum rather than commenting on this bug.
This work will be carried out on the goog_module branch. When working on migration, please branch from and submit PRs to merge back to goog_module, not develop.
Module Conversion Guide
Most of the work required to migrate from goog.provide to goog.module can be done on a module-by-module (and often file-by-file) basis.
The end goal is that each .js file will contain a single module, with a clearly defined interface, that can be imported independently of any other modules—except of course for that module's own dependencies, which it will import for its own internal use. Modules should normally only reexport identifiers from other modules when they are aggregating a number of submodules together (possibly for legacy compatibility reasons).
Since we're visiting every file in the project, we're also going to use this opportunity to begin the migration to ES6 and the new JavaScript styleguide.
Converting a single .js file
There are several mostly-independent steps needed to convert a single .js source file. In most cases it will be preferable to start conversion with leaf modules—i.e., ones which do not require any others—but within any given module that does have imports it will probably be easier to start by dealing with its goog.requires before dealing with its goog.provides.
Where possible you should present the conversion of a single .js file as a pull against the goog_module branch, consisting of four commits, as follows:
- Update file to ES6 const/let.
- Rewrite the
goog.providestatement asgoog.moduleand explicitly enumerate exports. - Rewrite
goog.requiresstatements and add missing requires. - Run
clang-formaton the whole file.
In situations where step 2 requires combining or splitting several related .js files (see details below) then it is appropriate that they be presented together in a single PR.
1. Update the file to ES6 const/let
[Details TBC but the plan is to use an automated tool convert all vars to let or const. We are not proposing to convert ES5 classes to ES6 class syntax at the present time.]
Suggested commit message: Migrate path/to/filename.js to ES6 const/let
Or use scripts/goog_module/convert-file.sh -c 1 path/to/filename.js.
(If you / your IDE does more than just const/let then adjust message accordingly.)
2. Rewrite the goog.provide statement as goog.module and explicitly enumerate exports
This is the larger part of the work and will require probably require the most thought.
When converted:
-
Each
.jsfile should begin with (and contain in total) a singlegoog.modulestatement. When loaded, the entire file will be evaluated in a local scope, as if it were surrounded by an IIFE:(function (exports) { /* File contents goes here. */ return exports; )({});
Any other module importing this one will get the return value of this IIFE (i.e., the value of
exports) as the return value ofgoog.require()—but note that the module is evaluated only once, and the return value is cached by the module loader. -
The
goog.modulestatement should be followed immediately by a call to declareLegacyNamespace:goog.module('foo.bar'); goog.module.declareLegacyNamespace();
This is a transition measure which has the effect of causing any symbols exported by this module to be accessible from the global scope by the same paths as they were previously—i.e., if module
foo.barexportsbazthen unmigrated code can still access it by:goog.require('foo.bar'); console.log(foo.bar.baz); // Works.
-
Anything that the module intends to export (classes, enums, functions, constants, and other symbols) should be named via a property on the (module-local)
exportsvariable—in most cases after being declared as a top-levelconst,classorfunction. Note that you will need to change any internal references as well. E.g.:goog.provide('MyProject.geometry'); /** @const */ MyProject.geometry.PI = 3.14; // Close enough. /** @param {number} radius @return number */ MyProject.geometry.diameterFromRadius = function(radius) { return MyProject.geometry.PI * radius; };
becomes:
goog.module('MyProject.geometry'); /** @const */ const PI = 3.14; // Close enough. exports.PI = PI; /** @param {number} radius @return number */ const diameterFromRadius = function(radius) { return PI * radius; }; exports.diameterFromRadius = diameterFromRadius;
We've decided to prefer
const funcName = function() {...tofunction funcName() {...because it prevents accidental redefinition of the symbol and it is more consistent with how other module-local identifiers are declared.In cases where when the module contains a single class you can (and for compatibility in most cases should) just export that class directly:
// ES5-style class: /** @constructor */ function MyClass(...) { ... }; MyClass.prototype.method = function() { ... }; exports = MyClass;
or, equivalently:
// ES6 class: class MyClass { ... method() { ... } ... }; exports = MyClass;
- This export-a-single-class pattern seems not to be a recommended pattern for new code but facilitates the migration by avoiding the necessity of making changes in other files where this module is imported. We have an open issue discussing changing this in the future.
-
Any top-level
@privateor otherwise internal identifiers should become top-level, module-local identifiers; they should not normally get marked@privatenor do their names end with underscore—but where it is necessary to export them for the time being in order to avoid breaking things, they should be exported under their original name and retain their access modifier tags. Access modifier tags remain with the main declaration rather than (as we initially tried) with the export. E.g.:goog.provide('MyModule'): /** @param {number} input @return {number} @public */ MyModule.publicThing = /* ... */; /** @param {number} input @return {number} @package */ MyModule.packageThing = /* ... */; /** @param {number} input @return {number} @private */ MyModule.supposedlyPrivateThing_ = /* ... */; /** @param {number} input @return {number} @private */ MyModule.actuallyPrivateThing_ = /* ... */;
becomes:
goog.module('foo.bar'); /** @param {number} input @return {number} */ const publicThing = /* ... */; exports.publicThing = publicThing; /** @param {number} input @return {number} @package */ const packageThing = /* ... */; exports.packageThing = packageThing /** @param {number} input @return {number} @private */ const supposedlyPrivateThing = /* ... */; exports.supposedlyPrivateThing_ = supposedlyPrivateThing /** @param {number} input @return {number} */ const actuallyPrivateThing = /* ... */;
Much of this process can be automated using scripts/goog_module/convert-file.sh -s 2 path/to/filename.js.
After converting the goog.provide statement and exports in this way you must run
npm run build:deps
to update tests/deps.js (or let npm test do this for you) after which the code should again continue to work as before. Make sure to include the updated deps.js in the commit.
Use npm start and npm test to verify that nothing is seriously broken before committing.
Suggested commit message: Migrate path/to/filename.js to goog.module
Or use scripts/goog_module/convert-file.sh -c 1 path/to/filename.js.
Notes:
-
The most important—and probably challenging!—part of this work will be clearly establishing what the public API of each module is.
-
Contrary to the conventional pattern when using
goog.provide, there should in general be few (if any) circumstances under which agoog.moduleadds new properties to an object it has imported from another module. (This does not prohibit modules from using some part of the Blockly API suchdefineBlocksWithJsonArrayto do so, however.) -
There are existing
.jsfiles that have multiplegoog.providestatements in them. They will need to be separated into separate.jsfiles each with a singlegoog.module. -
Conversely, there are cases where what ought to be a single module is spread over multiple
.jsfiles. This must be also be rectified.- The most straight-forward solution is just to concatentate them into a single
.jsfile. - Alternatively, where a top-level module can be created which imports and re-exports identifiers from a number of submodules.
Concatenation is probably the preferred solution, especially when the former multiple parts of a module share private identifiers which we do not wish the resulting module to export.
- The most straight-forward solution is just to concatentate them into a single
-
There are some
.jsfiles—notably the ones inblocks/—whose main purpose is not to export identifiers but to call functions on other modules in order to carry out some specific action (e.g., creating blocks). Such a module might not export anything.
3. Rewritegoog.requires statements and add missing requires
Anything required from another module should be given an explicitly name by assigning the return value of goog.require to a constant. For example, existing code of the form:
require('MyProject.MyClass');
var myObject = new MyProject.MyClass();should become:
const MyClass = require('MyProject.MyClass');
let myObject = new MyClass();Where importing a main export which was previously a module's default export, or where you need only a few particular identifiers from a module, you can import them directly:
const {Block} = require ('Blockly.Block');
const {RED, GREEN} = require(`MyProject.colours);However:
- Destructured exports cannot be stubbed/mocked in tests, so if a method needs to be stubbed/mocked the module it is defined in should not be destructured.
- Be sure to read and apply the rules in the (new) JavaScript Style Guide concerning how to name and order
goog.requires.
This step can mostly be automated by using scripts/goog_module/convert-file.sh -s 3 path/to/filename.js.
After converting all the goog.require statements in this way the code should continue to work as before.
Use npm start and npm test to verify that nothing is seriously broken before committing.
Suggested commit message: Migrate path/to/filename.js named requires
Or use scripts/goog_module/convert-file.sh -c 3 path/to/filename.js.
Notes:
- Be alert to clashes between the names imported via
goog.requiresand existing declarations in the file. It will be necessary to rename any clashing top-level identifiers (the import name takes precedence because it is mandated by the style guide); when a local variable shadows an imported name it is best to rename it too, even if the imported name isn't used in the scope of the shadowing variable. - There are known to be missing
goog.requirestatements in some parts of the codebase, so you will in some cases have to add these missing statements as you go along. - There will be cases where, when looking at chain of identifiers ("
foo.bar.baz.quux"), it is not clear where the name of the module ends and where the names of identifiers exported by that module begins—particularly if that module has not yet been converted togoog.modulesyntax. (This is one reason to prefer converting leaf modules first!)- Consult the source of the module being imported and then try to make as reasonable a guess as you can.
- It is unusual to refer to an identifier nested more than one level deep from an import—for example, in
foo.bar.baz.quuxit more likely that the module is or should be namedfoo.bar.bazandquuxis an identifier exported by than that the module is namedfoo.barandbazis the exported identifier for an object with a property namedquux.
4. Run clang-format on the whole file
Since we're touching everything, let's really touch everything. Two easy steps:
npx clang-format -i path/to/filename.js(orscripts/goog_module/convert-file.sh -s 4 path/to/filename.js) to format, andnpm run lintto verify that clang-format has not broken any of eslint's rules. (Better to find out now than when GitHub's CI catches it!)
Suggested commit message: clang-format path/to/filename.js
Or use scripts/goog_module/convert-file.sh -c 4 path/to/filename.js.
Submitting your Pull Request
There is a special pull request template for migration PRs; use it instead of the standard one.
General To Do List
This is a list of any and all other work that needs to be done to finish the migration.
First pass:
- Create
goog_modulebranch. - Fix mocha and generator tests to use
http-serverinstead offile:URLs. - Commit Update package-lock.json to lockfileVersion 2 #5038.
- FFWd
goog_modulebranch to tip ofdevelopbranch before merging any PRs into it. - Commit Preparation for goog.module transition: base.js, deps.js #5019.
- Provide PR to configure
eslintto accept ES6 input. - Provide PR template for migration PRs.
- Review contents of
blockly.js#5208
First pass cleanup:
- Update exports style in goog.module-migrated files #5133
- Remove or deprecate exported names ending in
_#5135
Further cleanup:
- refactor: Rename Blockly.connectionTypes to Blockly.ConnectionType #5407
- Finish migrating tests to goog.module #5443 (This does not need to be resolved in the near future.)
(Please edit this bug to add any outstanding work as required.)