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

refactor(tests): Update bootstrap.js to better support generator chunks #7171

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

cpcallen
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

There are some issues with how tests/bootstrap.js loads modules in compiled and uncompiled mode:

  • It does really relate goog.module IDs loaded in uncompressed mode vs. the compiled chunk file names loaded in compressed mode.
  • It relied on certain quirks of how the generator chunks worked—specifically, that you can goog.require('Blockly.JavaScript.all') to load the whole chunk via it's official entry point, but then do var {javascriptGenerator} = goog.module.get('Blockly.JavaScript') to get the javascriptGenerator object, since it is exported from both Blockly.JavaScript and Blockly.JavaScript.all. This prevents certain desirable refactorings of the generator modules.

Proposed Changes

Refactor bootstrap.js and bootstrap_helper.js to be able to deal with generator chunks. In particular for each chunk, specify:

  • The goog.module ID to goog.require() in uncompressed mode.
  • The script filename to load in compressed mode.
  • Where the chunk's UMD wrapper will save the export object when loaded as a script.
  • What global variable the chunk's export object should be saved in (if desired).
  • Any individual named exports to destructure to global variables.

This allows the bootstrap scripts to be slightly simpler while also being more flexible.

Test Coverage

No special manual testing changes required: any additional problems will become abundantly clear!

Refactor bootstrap.js and bootstrap_helper.js to be able to deal
with generator chunks.  In particular for each chunk, specify:

- The goog.module ID to goog.require() in uncompressed mode.
- The script filename to load in compressed mode.
- Where the chunk's UMD wrapper will save the export object when
  loaded as a script.
- What global variable the chunk's export object should be saved in
  (if desired).
- Any individual named exports to destructure to global variables.

This allows the bootstrap scripts to be slightly simpler while
also being more flexible.
@cpcallen cpcallen requested a review from a team as a code owner June 15, 2023 18:16
@cpcallen cpcallen requested a review from NeilFraser June 15, 2023 18:16
@BeksOmega BeksOmega assigned BeksOmega and unassigned NeilFraser Jun 15, 2023
@BeksOmega BeksOmega requested review from BeksOmega and removed request for NeilFraser June 15, 2023 18:23
@cpcallen cpcallen merged commit 817ffab into google:develop Jun 15, 2023
@cpcallen cpcallen deleted the refactor/7086/bootstrap branch June 15, 2023 20:03
cpcallen added a commit to cpcallen/blockly that referenced this pull request Jun 18, 2023
Due to errors in PRs google#7171 and 7173 (and the author's failure to do
enough local testing before submitting those PRs), compressed mode
loading was broken in the playgrounds.  Fix this by:

- Fix a typo in bootstrap.js ("Blocky" -> "Blockly").
- Updating the chunks definitions build_tasks.js to use the new
  variables we expect to contain generator exports objects.
cpcallen added a commit that referenced this pull request Jun 20, 2023
Due to errors in PRs #7171 and 7173 (and the author's failure to do
enough local testing before submitting those PRs), compressed mode
loading was broken in the playgrounds.  Fix this by:

- Fix a typo in bootstrap.js ("Blocky" -> "Blockly").
- Updating the chunks definitions build_tasks.js to use the new
  variables we expect to contain generator exports objects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants