Uh oh!
There was an error while loading. Please reload this page.
refactor(generators): Restructure generator modules to contain side effects - #7173
Merged
cpcallen merged 9 commits intoJun 20, 2023
Merged
Conversation
Move the LangGenerator definitions into their respective subdirectories and add a _generator suffix to their filenames, i.e. generators/javascript.js becomes generators/javascript/javascript_generator.js. This is to keep related code together and allow the `lang/all.js` entrypoints to be moved to the top level generators/ directory. No goog module IDs were changed, so playground and test code that accesses this modules by filename does not need to be modified.
- Move the entrypoints in generators/*/all.js to correspondingly-named files in generators/ instead—i.e., generators/javascript/all.js becomes generators/javascript.js. - Update build_tasks.js accordingly.
These were inadvertently omitted from RaspberryPiFoundation#7161 and RaspberryPiFoundation#7162, respectively.
- Move declaration of <lang>Generator instance from generators/<lang>/<lang>_generator.js to generators/<lang>.js. - Move .addReservedWords() calls from generators/<lang>/*.js to generators/<lang>.js - Modify generators/<lang>/*.js to export block generator functions individually, rather than installing on <lang>Generator instance. - Modify generators/<lang>.js to import and install block generator functions on <lang>Generator instance.
Where these tests needed block generator functions preinstalled they should have been importing the Blockly.<Lang>.all module. Where they do not need the provided block generator functions they can now create their own empty <Lang>Generator instances.
- Fix a malformation in previous entries that was not detected by the renaming file validator test. - Add entries describing the work done in this and related recent PRs.
BeksOmega
approved these changes
Jun 16, 2023
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts: # scripts/migration/renamings.json5
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Fix a search-and-replace error in renamings.json5 - Fix an incorrect-but-usable import in generator_test.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The basics
npm run formatandnpm run lintThe details
Resolves
Fixes#7086
Proposed Changes
Implements proposal 2 (option 2A) of #7086. In particular:
generators/<lang>.jsare moved togenerators/<lang>/<lang>_generator.js. Code creating<lang>Generatorinstances is removed from these file.generators/<lang>/*.js(excluding<lang>_generator.js) are modified to declare and individually export per-block generator functions, not referencing any particularCodeGeneratorinstance. Calls to<lang>Generator.addReservedWords()re replaced with comments noting the reserved words needed by the generator functions in that file.generators/<lang>/all.jsare moved togenerators/<lang>.jsand modified to:generators/<lang>/<lang>_generator.js.<Lang>Generatorinstance named<lang>Generator.<langGenerator.addReservedWords()to add reserved words mentioned in the block generator files (if any).generators/<lang>/*.json<lang>Generator.forBlock.These file remain the entrypoint for the
<lang>_compressed.jschunks.Additionally, this PR:
chunksdeclaration inbuild_tasks.jsas accordingly.scripts/migration/renamings.json5to record these updates plus update references fromBlockly.<Lang>to<lang>.<lang>Generator(only applicable, and only effective, on code that imports generators as scripts.exportstatements for someclass <Lang>Generators.Behaviour Before Change
Generators work.
Behaviour After Change
Unchanged.
Reason for Changes
See #7086.
Test Coverage
Passes
npm test. No changes to manual test procedures expected.Documentation
No documentation updates needed. This PR is an internal-only refactor.
Additional Information
This change paves the way for better tree-shaking of generator functions, though the individual generator functions are not yet exposed in in the compiled chunks. Projects that compile Blockly from source (such as Blockly Games) should be able to take advantage of this change immediately.
Notes for Reviewer
Because of the file moves, it will probably be easier to review this PR commit-by-commit.