Proposal: Library generator publication changes + supporting change to CodeGenerator

[cpcallen]

This is the third of three issues created to discuss proposed changes to generators (and perhaps track implementation of those proposals, if agreed). This issue concerns proposed changes to the way the generators in generators/ are published (to form the basis of a standard for the publication of generators as plugins), plus a small breaking change to the CodeGenerator class in core/generator.ts to support this. This proposal assumes that the principle parts of the proposals in #7084 and #7085 have been adopted, but is only partially contingent on that.

In code examples in these bugs, the word "language" / "Language" / "LANGUAGE" should be read as a metasyntactic variable standing in for some particular language like JavaScript.

Background

At the moment, the way per-block generator functions are created is via side effects (assigning to a property on a CodeGenerator instance), and ties them directly to a particular CodeGenerator instance (by calling other methods on that specific instance):

languageGenerator['block_type'] = function(block) {
  // Get code for some argument input:
  const argumentCode = languageGenerator.valueToCode(block, 'INPUT_NAME', languageGenerator.ORDER_FOO);
  const code = /* some code + */ argumentCode /* + more code */;
  const outputOrder = languageGenerator.ORDER_BAR;
  return [code, outputOrder];
};

This means that it is difficult to load just specific generator functions, and it is not generally safe to copy generator functions from one CodeGenerator instance to another.

Proposal 1: Pass this CodeGenerator when calling generator functions

Option 1A: Pass as parameter

The CodeGenerator instance could be passed as a parameter. For backward compatibility it should be the second parameter:

languageGenerator['block_type'] = function(block: Block, generator: LanguageGenerator) {
  // Get code for some argument input:
  const argumentCode = generator.valueToCode(block, 'INPUT_NAME', generator.ORDER_FOO);
  const code = /* some code + */ argumentCode /* + more code */;
  const outputOrder = generator.ORDER_BAR;
  return [code, outputOrder];
};

Option 1B: Pass as this

Alternatively the generator instance it could (via Function.prototype.call) be passed as this:

languageGenerator['block_type'] = function(this: LanguageGenerator, block: Block) {
  // Get code for some argument input:
  const argumentCode = this.valueToCode(block, 'INPUT_NAME', this.ORDER_FOO);
  const code = /* some code + */ argumentCode /* + more code */;
  const outputOrder = this.ORDER_BAR;
  return [code, outputOrder];
};

Breaking Changes

This would be a breaking change in CodeGenerator, which currently passes block as this as well as the first parameter, but:

• The block has been supplied as the first argument to generator functions for nearly ten years now, so it is unlikely that many generators in the wild are using this instead of the block parameter.
• This approach is has the advantage of making these functions seem more like methods on a CodeGenerator instance—which they actually have been until now (though that may change; see Proposal: CodeGenerator per-block-type generator function dictionary #7084).

Proposal 2: Restructure generator modules to contain side effects

At present each language generator consists of three groups of modules:

1. generators/language.js creates and exports a CodeGenerator instance (unstylishly named Language, despite being an instance and not a class)
2. generators/language/*.js (excluding all.js) add methods to the Language instance object.
3. generators/language/all.js requires all of the above and reexports the Language object. This is the entry point for the language_compressed.js chunk.

Instead, this alternative structure is proposed:

1. generators/language/generator.ts (replacing generators/language.js) will declare and export CodeGenerator subclass named LanguageGenerator, possibly along with additional exports such as an Order enum (see proposals in Proposal: Per-language subclasses of CodeGenerator in generators/ #7085)—but not create a languageGenerator instance.
2. generators/language/*.ts (excluding generator.js) will declare and export per-block generator functions, not referencing any particular CodeGenerator instance.
3. generators/language.ts (replacing generators/language/all.js) will import the above, create and export a LanguageGenerator instance named languageGenerator, and add the individual generator functions to that instance's dictionary. This file will be the entrypoint for the language_compressed.js chunk.

Rationale for the change

• Users of the compiled chunk will continue to get a fully-built and ready-to-use CodeGenerator instance, largely obviating breaking changes due to this restructuring.
• Projects that build Blockly themselves (like Blockly Games) can pick and choose exactly what individual generator functions to include in each LanguageGenerator instance, and can have more than one such instance—e.g. with different settings and different blocks supported.
• We can in due course add additional entry points (chunks) to allow users of our published NPM packages to do the same.
• This tripartite arrangement, with both a set of pick-and-choose entry points as well as a batteries-included entry point, seems like a good structure for publishing generators as plugins.

Option 2A: Export block generator functions individually

In this option each block generator is exported from its module separately:

export block_type_1(block: Block, generator: LanguageGenerator) { /* ... */ return code; }
export block_type_2(block: Block, generator: LanguageGenerator) { /* ... */ return code; }
// ...

This option is probably better for ensuring that tree-shaking is successful (where relevant for building minimal compiled bundles), but JS/TS language rules would restrict the block types to valid identifiers (and prohibit 'default', 'this' and other language keywords), and violates the styleguide rules on naming (and this will likely require eslint exceptions).

Option 2B: Export block generators in a dictionary object

In this option all the block generators are exported as properties on a single dictionary object:

export const blockGenerators = {
  'block_type_1' : function(block: Block, generator: LanguageGenerator) { /* ... */ return code; },
  'block_type_2': function(block: Block, generator: LanguageGenerator) { /* ... */ return code; },
  // ...
};

This imposes no additional restrictions on block types, but requires that we choose an arbitrary export name (here 'blockGenerators', but suggestions welcome), and may make tree shaking harder. (It could also be construed as being contrary to the styleguide prohibition against namespace objects.)

Proposal 3: Rename the <script> generator entrypoints [UPDATED]

Currently, the recommended way to import e.g. the JavaScript CodeGenerator instance is to import {javascriptGenerator} from 'blockly/javascript';, but when the javascript_compressed.js chunk is loaded via a <script> tag, the javascrtiptGenerator named export (only) is made available via globalThis.Blockly.JavaScript. This was done for backwards compatibility with the original goog.provides implementation, but creates some problems:

• It creates an naming inconsistency depending on whether loading is being done by <script> or import/require, creating some confusion when discussing generators in our documentation and examples and with developers in the Blockly forum.
• It does not leave us an obvious place to make available additional exports from these modules, such as LanguageGenerator class constructors, Order enums, or individual generator functions.
• It depends on modifying the Blockly module (exports) object in a way that would normally be prohibited, and is only possible because we have transpiled it from an ESM to a UMD/CJS module.

It is proposed that new bindings be introduced an the existing ones eventually removed after a suitable notice/transition period.

Option 3A: Use a Blockly.<language>.* namespace

Make all exports from the entrypoint module (currently generators/<language>/all.js, to be renamed to generators/<language>.ts under proposal 2, above) available via globalThis.Blockly.<language>.*:

• The existing javascriptGenerator object, presently accessible as Blockly.JavaScript, would be exported as Blockly.javascript.javascriptGenerator.
• A JavascriptGenerator class could be exported as Blockly.javascript.JavascriptGenerator.
• An Order enum could be exported as Blockly.javascript.Order.

This only partially fixes the naming inconsistency and does not avoid modifying Blockly, but avoids any namespace pollution.

Option 3B: Use a <language>.* namespace

Make all exports from the entrypoint module available via globalThis.<language>.*:

• The existing javascriptGenerator object, presently accessible as Blockly.JavaScript, would be exported as javascript.javascriptGenerator.
• A JavascriptGenerator class could be exported as javascript.JavascriptGenerator.
• An Order enum could be exported as javascript.Order.

This makes <script src="language_compressed.js"> roughly equivalent to import * as language from 'blockly/language';, avoids modifying Blockly, and creates only minimal namespace pollution.

Option 3C: Add all exports directly to the global scope

Make all exports from the entrypoint module available via globalThis.*:

• The existing javascriptGenerator object, presently accessible as Blockly.JavaScript, would be exported as javascriptGenerator.
• A JavascriptGenerator class could be exported as JavascriptGenerator.
• An Order enum could be exported as Order. (We might wish to chose a more specific name for such exports!)

This makes <script src="language_compressed.js"> roughly equivalent to import {languageGenerator, LanguageGenrator, /* ... */} from 'blockly/language';, allowing us to consistently use a bare javascriptGenerator in our documentation and examples without causing confusion. It avoids modifying the Blockly module object, but creates considerable namespace pollution—including the potential for clashes between different generators.

Breaking Change

Options 3B and 3C introduce new bindings into the global scope, creating the possibility of a clash with any existing developer use of those bindings.

The eventual removal of Blockly.JavaScript et al. will obviously break any code that imports generators via <script> that has not updated to use the new names.

Additional Info

Note that this proposal only affects loading via <script>, and would be moot if we decide to publish Blockly as ESM rather than UMD/CJS modules: in that case loading Blockly would necessarily always be done via import.

[cpcallen]

I recommend we adopt all three proposals.

For proposal 1, I have an aesthetic preference for option 1B but concede that option 1A may be clearer and avoids the (I think small but non-zero) possibility of breaking existing generator functions.

For proposal 2 I have no strong preference between the options.

For proposal 3 I recommend option 3B; it seems like the best compromise between the various goals.

[BeksOmega]

At the moment, the way per-block generator functions are created... ties them directly to a particular CodeGenerator instance. This means that... it is not generally safe to copy generator functions from one CodeGenerator instance to another.

I would say that in general, it is not safe to copy block generator functions from one CodeGenerator to another regardless of whether they are created in a way that ties them to a particular CodeGenerator or not. E.g. javascript code looks different from python code, so we can't expect to copy the block-code generator from oneCodeGenerator to another.

As such, I don't really see the advantage of Proposal 1. Is this just a prereq for the other proposals?

Wrt proposal 2: The restructuring sgtm. I prefer method 1 for the restructuring because this seems better if you do just want to import individual blocks. However, I'm confused about how you want to add these methods to the dictionary. Are you going to re-key all of the functions?

E.g:

import {controls_if} from './controls.ts';
import {JavaScriptGenerator} from './generator.ts';

JavaScriptGenerator.blocksGenerators['controls_if'] = controls_if;
// etc...

If we want external developers to actually be able to import / use these individually, forcing them to do that seems suboptimal. But I don't really have a better suggestion.

Wrt proposal 3: Had we already discussed this when we were renaming the modules? Do you have context for what the conclusion was previously?

[cpcallen]

I would say that in general, it is not safe to copy block generator functions from one CodeGenerator to another regardless of whether they are created in a way that ties them to a particular CodeGenerator or not. E.g. javascript code looks different from python code, so we can't expect to copy the block-code generator from oneCodeGenerator to another.

Ahh, no sorry, you misunderstand: I am speaking only of copying from one instance to another of the same language, not between language.

As such, I don't really see the advantage of Proposal 1. Is this just a prereq for the other proposals?

It is a prerequisite to proposal 2, and more broadly to the ability to define and publish generator function without reference to a specific CodeGenerator instance.

[cpcallen]

This proposal was discussed at our weekly team meeting on 2023-05-16, but no decisions were recorded.