refactor!: Remove remaining use of goog.module.declareLegacyNamespace.
#6254
Conversation
This file contains 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
- Use TSC_OUTPUT_DIR to find goog/goog.js when suppressing warnings. - Remove unnecessary trailing semicolons.
Remove the call to goog.module.declareLegacyNamespace from Blockly.libraryBlocks. This entails: - Changes to the UMD wrapper to be able to find the exports object. - Changes to tests/bootstrap_helper.js to save the exports object in the libraryBlocks global variable. - As a precaution, renaming the tests/compile/test_blocks.js module so that goog.provide does not touch Blockly or Blockly.libraryBlocks, which may not exist / be writable.
We need to convert the generators to named exports. For backwards compatibility we still want e.g. Blockly.JavaScript to point at the generator object when the chunk is loaded using a script tag. Modify chunkWrapper to honour a .reexportOnly property in the chunks table and generate suitable additional code in the UMD wrapper.
- Export the JavaScript generator object as javascriptGenerator from the Blockly.JavaScript module(generators/javascript.js). - Modify the Blockly.JavaScript.all module (generators/javascript/all.js) to reexport the exports from Blockly.JavaScript. - Update chunk configuration so the generator object remains available as Blockly.JavaScript when loading javascript_compressed.js via a <script> tag. (N.B. it is otherwise necessary to destructure the require / import.) - Modify bootstrap_helper.js to store that export as window.javascriptGenerator for use in test code. - Modify test code to use javascriptGenerator instead of Blockly.JavaScript. - Modify .eslintrc.json so that javascriptGenerator is allowed as a global in test/. (Also restrict use of Blockly global to test/.) N.B. that demo code in demos/code/code.js uses <script> tag loading and so will continue to access Blockly.JavaScript.
Remove the goog.module.declareLegacyNamespace calls from the generators. This turns out to have the unexpected side-effect of causing the compiler to rename the core/blockly.js exports object from $.Blockly to just Blockly in blockly_compressed.js - presumably because it no longer needs to be accessed in any subsequent chunk because they no longer add properties to it. This requires some changes (mainly simplification) to the chunkWrapper function in build_tasks.js.
So easy to do _now_: just need to: - Make sure the UMD wrapper for the first chunk knows where the exports object is. - Use that same value to set the Blockly.VERSION @define. - Have bootstrap_helper.js set window.Blockly to the exports object. - Fix tests/compile/test_blocks.js to not assume a Blockly global variable, by converting it to a goog.module so we can use a named require.
cpcallen
added
component: build process
breaking change
BeksOmega
approved these changes
Jun 30, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Prerequisite to #5904 and #5857.
Proposed Changes
goog.module.declareLegacyNamespacecalls. (There was one for each chunk entry point.)tests/to keep everything working.Behaviour Before Change
Loading the library blocks or a generator module/chunk creates a new property on
Blockly(e.g..,Blockly.JavaScript).Behaviour After Change
Such a load only creates a new property on
Blocklyif it is loaded from the<chunk>_compressd.jsfile using a<script>tag.Reason for Changes
Removing
goog.module.declareLegacyNamespaceis a prerequisite to converting agoog.moduleinto an ES or TS module.Test Coverage
npm test.tests/playground.htmlandtests/multi_playground.jsload correctly and appear to be fully functional.developand will be fixed separately.Documentation
We need to make sure that our documentation and release notes indicate that if you need to use a generator you now need to use a named import:
Additional Information
As of this PR, the backward-compatibility
Blockly.JavaScriptproperties are only created when loading the corresponding chunk using a<script>tag. If desired, we can (perhaps in a separate PR) modify the UMD wrapper so that these properties are always created, even when loading using (e.g.) a side-effects-onlyimport 'blockly/javascript'.