Migrate Blockly from goog.provide to goog.module

[cpcallen]

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:

1. Update file to ES6 const/let.
2. Rewrite the goog.provide statement as goog.module and explicitly enumerate exports.
3. Rewritegoog.requires statements and add missing requires.
4. Run clang-format on 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 .js file should begin with (and contain in total) a single goog.module statement. 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 of goog.require()—but note that the module is evaluated only once, and the return value is cached by the module loader.

• The goog.module statement 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.bar exports baz then 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) exports variable—in most cases after being declared as a top-level const, class or function. 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() {... to function 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 @private or otherwise internal identifiers should become top-level, module-local identifiers; they should not normally get marked @private nor 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 a goog.module adds new properties to an object it has imported from another module. (This does not prohibit modules from using some part of the Blockly API such defineBlocksWithJsonArray to do so, however.)

• There are existing .js files that have multiple goog.provide statements in them. They will need to be separated into separate .js files each with a singlegoog.module.

• Conversely, there are cases where what ought to be a single module is spread over multiple .js files. This must be also be rectified.

  • The most straight-forward solution is just to concatentate them into a single .js file.
  • 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.

• There are some .js files—notably the ones in blocks/—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.requires and 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.require statements 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 to goog.module syntax. (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.quux it more likely that the module is or should be named foo.bar.baz and quux is an identifier exported by than that the module is named foo.bar and baz is the exported identifier for an object with a property named quux.

4. Run clang-format on the whole file

Since we're touching everything, let's really touch everything. Two easy steps:

1. npx clang-format -i path/to/filename.js (or scripts/goog_module/convert-file.sh -s 4 path/to/filename.js) to format, and
2. npm run lint to 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_module branch.
• Fix mocha and generator tests to use http-server instead of file: URLs.
  • Issue: Mocha tests fail when loading goog.modules #5030, fix: Add flag to webdriverio options #5035.
• Commit Update package-lock.json to lockfileVersion 2 #5038.
• FFWd goog_module branch to tip of develop branch before merging any PRs into it.
• Commit Preparation for goog.module transition: base.js, deps.js #5019.
• Provide PR to configure eslint to 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.)

[cpcallen]

A question came up in chat:

How should we be converting to provide -> module namespace? E.g. for goog.provide('Blockly.utils.object'), should that become goog.module('Blockly.utils.object') or something else?

Answer: yes, in general the module name should be the same as the formerly-provided name.

Possible exceptions:

1. If a file had just one export:

   goog.provide('foo.bar');
   foo.bar.baz = {x: ..., y: ...};
   

   then we might want to export the properties of that object directly:

   goog.module('foo.bar.baz');
   const x = ...:
   const y = ...:
   exports = {x, y};
   

   But probably only if x and y were unrelated (i.e., not enum values or the like).

2. Contrarily, if a file provided a class,:

   goog.provide('foo.bar.baz');
   /** @constructor */
   foo.bar.baz = function() {...};
   

   and there was no file that goog.provides the parent namespace (foo.bar), then we could change it to avoid having it be the 'default export':

   goog.module('foo.bar');
   class baz { ... }
   exports = {baz};
   

I think such cases will be rare, and changes like this will require changing the code at any place the module is imported, so are probably best left for a second pass.

[cpcallen]

There have been a few cases come up where there was uncertainty about what should be exported an how. One common situation that has caused doubt is where a module provides a class constructor as well as some other dubiously-related items exported via static properties on that constructor. For instance, from #5047, core/utils/coordinate.js originally looked like:

goog.provide('Blockly.utils.Coordinate');

Blockly.utils.Coordinate = function(x, y) { /* omitted */};

/**
 * Compares coordinates for equality.
 * @param {?Blockly.utils.Coordinate} a A Coordinate.
 * @param {?Blockly.utils.Coordinate} b A Coordinate.
 * @return {boolean} True iff the coordinates are equal, or if both are null.
 */
Blockly.utils.Coordinate.equals = function(a, b) { /* omitted */ };

Blockly.utils.Coordinate.distance = function(a, b) {
  …

In this case equals makes sense as a static method on the Coordinate constructor:

• It takes two Coordinates as parameters, so it's obviously closely related to the Coordinate type.
• It can't be replaced by a Coordinate.prototype.equals(other) method because as typed it tolerates either a or b (or both) being null, while a.equals(b) will throw TypeError if a is null.

It is therefore totally appropriate to leave Coordinate.equals as a static method, and just export the single object Coordinate, giving a revised file that looks like:

goog.module('Blockly.utils.Coordinate');
goog.module.declareLegacyNamespace();

const Coordinate = function(x, y) { /* omitted */ };

Coordinate.equals = function(a, b) { /* omitted */ };

Coordinate.distance = function(a, b) { /* omitted */ };
// etc.

exports = Coordinate;

A good clue that this situation applies is when the package path ends with a capitalised identifier ("….Coordinate")—though there will doubtless be cases (including "Blockly") where things have been misnamed.

A related situation are enums, where one might quite reasonably have e.g.

goog.provides('TrainStatus');

/** @enum {number} */
const TrainStatus = {
  early: 0;
  onTime: 1;
  late: 2;
}

exports = TrainStatus;

From the point of view of code importing this module it could just as easily have been written:

goog.provides('trainStatus');

const EARLY =  0;
const ON_TIME = 1;
const LATE = 2;

exports = {EARLY, ON_TIME, LATE};

Except now there's nowhere to put the @enum tag, making it difficult to use in type annotations.

On the other hand: if you find a module which exports a class constructor that has some properties on it that do not seem to be too closely related to the class—for instance, something like a Circle class that has a Circle.PI = 3.14, where .PI is neither a method taking or returning Circles, nor is it a distinguished/singleton Circle instance—then this would be a good candidate for doing some API restructuring once we have finished the first pass of file-by-file migrations.

Please add any such files to the appropriate list in issue #5073 (where the "triage" list is the appropriate list if you're not sure!)

[alschmiedt]

@cpcallen and @rachel-fenichel can we close this issue? I think the only piece left is converting the tests to goog.module. Which I believe we decided does not need to be done in any hurry.

[alschmiedt]

Closing this issue since all of the remaining work is being tracked in other issues.