Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: make block modules side-effect free #5875

Open
2 of 4 tasks
cpcallen opened this issue Jan 13, 2022 · 5 comments
Open
2 of 4 tasks

Proposal: make block modules side-effect free #5875

cpcallen opened this issue Jan 13, 2022 · 5 comments
Assignees
Labels
breaking change Used to mark a PR or issue that changes our public APIs. component: library blocks issue: feature request Describes a new feature and why it should be added
Milestone

Comments

@cpcallen
Copy link
Contributor

cpcallen commented Jan 13, 2022

Background

At the moment, the blocks modules (Blockly.blocks.*, in blocks/) generally do not have any exports; instead, importing one causes the created blocks definitions to be loaded into the block definitions dictionary (Blockly.Blocks, or more specifically the Blocks export of the Blockly.blocks module) as a side effect. Various extensions are also registered (again as a side-effect) via the register* exports of the Blockly.Extensions module.

Additionally, library block modules are normally imported via a single import 'blockly/blocks' statement, which imports the Blockly.blocks.allmodule, which in turn imports the rest of the individualBlockly.blocks.*` modules.

This creates some problems for developers:

  • Loading blocks via import 'blockly/blocks' is all-or-nothing: only developers using the Closure module system can use goog.require to load only a specific module (e.g. Blockly.blocks.loops).
  • There is no way to load only certain blocks from an individual module. At best you must load them all and then delete unwanted ones from Blockly.Blocks (as well as unregister any unwanted extensions).
  • There is at present no way for developers who use ES Module import statements to access exports from individual block module (e.g., the loopTypes export from Blockly.blocks.loops)—see There is no mechanism to register custom loops anymore since the 2021 Q4 release #5819.

Proposed solution

Refactor the block modules in several steps, as follows:

  1. Add a standard set of exports to the block modules:

    • blocks: a dictionary object (@type {!Object<string, !Object>}), keyed by block type, of the block definitions that that module creates.
    • extensions: a dictionary object (@type {!Object<string, !Object>}) keyed by extension name, of the extensions registered by the module.
    • Any other module-specific exports (e.g., loopTypes).

    The blockly.blocks.all module will provide blocks and extensions dictionaries that include all the entries form all of the individual block modules. Additionally, it will re-export the exports objects from each of those modules using the corresponding names (e.g. colour, lists, loops, etc.)

    This will enable importers to see the full set of blocks (and extensions) that are loaded, as well as access any other exports provided by individual block modules (e.g., loops.loopTypes).

    This will be a breaking change to the exports provided by importing blockly/blocks:

    • At present blockly/blocks provides a default export of the whole Blockly.Blocks dictionary.
    • As proposed above, it will instead provide a named export of a dictionary containing only the blocks defined in blocks/ (and not any which might have been added to Blockly.Blocks before or after blockly/blocks was imported), in addition to other named exports.
      …but it is not believed that very many external developers will be impacted by it.
  2. Optional intermediate step: refactor block modules so that rather than creating block definitions in the Blockly.Blocks dictionary and then extracting them for the blocks export, they are created in the blocks exported dictionary and then added to Blockly.Blocks—and similarly for extensions. This will not be a breaking change.

  3. Refactor the block modules so that they do not by default add block (and extension) definitions to Blockly. Instead, provide a method on Blockly to copy block definitions from a provided dictionary into Blockly.Blocks upon request. This will be a breaking change necessitating developers to manually request block and extension definition installation—but thereby allowing them to choose exactly which definitions they wish to install.

    For convenience, an exported function (perhaps registerAll or 'installBlocks) could be provided on each block module to facilitate making the correct install the block definitions and register extensions from that module.

Alternatives

Some alternatives to the proposal above (which have not yet been fully considered):

  • Do only step 1 above, leaving the modules as they presently are, such that they install block definitions and register extensions as a side effect.
  • Instead of having blocks and extensions as separate named exports, they could be encapsulated into a single named export (possibly of a type such as BlocksAndExtensions with instance properties .blocks and .extensions) which could be passed to an install-and-register function on Blockly. This might slightly ease the transition for developers.

Current status

  • Add a standard export for blocks.
  • Add a standard export for extensions.
  • Refactor block modules so they are created in the exported blocks dictionary and then installed in Blockly.Blocks.
  • Refactor block modules so they do not by default install blocks in Blockly.Blocks. (Breaking change!)
@cpcallen cpcallen added issue: feature request Describes a new feature and why it should be added component: library blocks issue: triage Issues awaiting triage by a Blockly team member labels Jan 13, 2022
@cpcallen
Copy link
Contributor Author

@rachel-fenichel I believe the above summarises our conversation yesterday, with a few additional ideas that have occurred to me in the mean time.

Input from the Blockly team as well as external developers welcome.

@BeksOmega
Copy link
Collaborator

Related to step 3, should the block plugins provided in samples also be changed to have some kind of installation step? I'm not sure if fixing the inconsistency is worth the extra work or not.

@gonfunko gonfunko added breaking change Used to mark a PR or issue that changes our public APIs. and removed issue: triage Issues awaiting triage by a Blockly team member labels Jan 14, 2022
@cpcallen
Copy link
Contributor Author

Related to step 3, should the block plugins provided in samples also be changed to have some kind of installation step?

Yes: if we make the blocks modules in blocks/ side effect free then the ones in samples should be modified similarly.

@maribethb
Copy link
Contributor

I think we can do this without introducing a breaking change this quarter, and would greatly prefer to keep to that goal. Let's provide new modules that behave as described (can be individually imported, no side effects, etc.) while maintaining the old modules that keep the current behavior for now. That gives developers some time to move over to the new format without mandating they switch immediately.

The change to the generator imports was actually pretty disruptive/painful to some partners and I'd rather not do another one right on the heels of it. Let's wait a while longer and batch up the breaking changes until we hit something that's unavoidable to break, or we have a big enough batch.

@cpcallen
Copy link
Contributor Author

I think we can do this without introducing a breaking change this quarter, and would greatly prefer to keep to that goal. Let's provide new modules that behave as described (can be individually imported, no side effects, etc.) while maintaining the old modules that keep the current behavior for now. That gives developers some time to move over to the new format without mandating they switch immediately.

Yes, I concur. That's basically what we've done with generators. I think for both blocks and generators we want to provide one or more new import entry points (maybe something like blockly/blocks/<category> and generators/<lang>/<category> that are no-install, and keep the existing entry points (blockly/blocks and blockly/<lang>) batteries-included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. component: library blocks issue: feature request Describes a new feature and why it should be added
Projects
None yet
Development

No branches or pull requests

5 participants