-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix(tests): Fix bootstrapping of generators in compressed mode #6703
Merged
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
Previously we had code that would, in uncompiled mode, make sure that javascriptGenerator etc. were set to the corresponding module exports object (by fetching it with goog.module.get), but in compressed mode we made no effort to set (e.g.) javascriptGenerator to Blockly.JavaScript, which is where that export actually appears in the namespace tree when loading the chunk via a <script> tag. This commit introduces two new configuration options to bootstrap.js (preconfigured with defaults that should work in most cases) mapping global variable names to their corresponding module identifiers, and modifies the code in bootstrap_helper.js to set global variables appropriately in both uncompressed and compressed modes.
BeksOmega
approved these changes
Dec 14, 2022
maribethb
added a commit
to maribethb/blockly
that referenced
this pull request
Jan 12, 2023
google#6703)" This reverts commit f528ecc.
maribethb
added a commit
to maribethb/blockly
that referenced
this pull request
Jan 13, 2023
…ssed mode (google#6703)"" This reverts commit 09c845e.
cpcallen
added a commit
to cpcallen/blockly
that referenced
this pull request
Feb 8, 2023
Prior to PR google#6818, circular imports resulted in the debug module loader (in closure/goog/base.js) failing to record the goog.module ID of most modules that were involved in the cycle, and in particular of the Blockly.Xml module. This had secondary fallout that resulted in library blocks modules being loaded in the wrong order. A kludge was introduced in PR google#6703 that worked around this problem by making sure that window.Blockly was set, allowing the modules loaded out-of-order to still work. Now that we have removed all remaining circular dependencies there is no need for the kludge, since all module IDs are properly recorded and modules are loaded in the correct order.
4 tasks
cpcallen
added a commit
that referenced
this pull request
Feb 8, 2023
* chore(tests): Remove circular import loading issue kludge Prior to PR #6818, circular imports resulted in the debug module loader (in closure/goog/base.js) failing to record the goog.module ID of most modules that were involved in the cycle, and in particular of the Blockly.Xml module. This had secondary fallout that resulted in library blocks modules being loaded in the wrong order. A kludge was introduced in PR #6703 that worked around this problem by making sure that window.Blockly was set, allowing the modules loaded out-of-order to still work. Now that we have removed all remaining circular dependencies there is no need for the kludge, since all module IDs are properly recorded and modules are loaded in the correct order. * chore(build): Remove exclude for non-existent core/blockly.js There was a transitional period where we had both core/blockly.ts and core/blockly.js, and wished to exclude the latter from tsc's input, but the latter file was deleted (and inadvertently restored, then re-deleted) some time ago.
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 format
andnpm run lint
The details
Resolves
Broken generators in (basic) playground.
Proposed Changes
Previously we had code that would, in uncompiled mode, make sure that
javascriptGenerator
etc. were set to the corresponding module exports object (by fetching it withgoog.module.get
), but in compressed mode we made no effort to set (e.g.)javascriptGenerator
toBlockly.JavaScript
, which is where that export actually appears in the namespace tree when loading the chunk via a<script>
tag.This commit introduces two new configuration options to
bootstrap.js
(preconfigured with defaults that should work in most cases) mapping global variable names to their corresponding module identifiers, and modifies the code inbootstrap_helper.js
to set global variables appropriately in both uncompressed and compressed modes.Behaviour Before Change
javascriptGenerator
et al. are only set in uncompressed mode.Behaviour After Change
javascriptGenerator
et al. are only set in both uncomrpessed and compressed mode.Additional Information
This was supposed to be a fix for #6597, but in fact the problems with advanced playground are more thorny, because (as used in
tests/playgrounds/advanced.html
) it actually ends up using two separate copies of Blockly simultaneously.