From ac2d68961821b36ecf1bc26282b1c695579df797 Mon Sep 17 00:00:00 2001
From: Christopher Allen <cpcallen+git@google.com>
Date: Mon, 13 Jun 2022 23:50:23 +0100
Subject: [PATCH] fix(tests): Rewrite bootstrap sequencing code

An earlier commit modified the generated <script> to use
goog.addDependency to trick the debug module loader into loading
.additionalScripts (via goog.bootstrap), but it turns out there is
a small problem: scripts like msg/messages.js have undeclared
dependencies on the Blockly module, and without a call to
goog.require('Blockly') in them they can end up being run before
the Blockly module is fully loaded.

(This problem only occurs when there are ES Modules, rather than
merely goog.modules, in the mix.)

Fix this by adding a script, bootstrap_helper.js, to be loaded
options.requires and any options.additionalScripts that makes an
explicit call to goog.require for each of option.requires.

Also refactor the code so that instead of generating a loop which
calls goog.addDependency, we generate the addDependency calls
directly.  This makes debugging a bit easer as we can use the browser's
dev tools to inspect the generated calls in the DOM tree.
---
 tests/blockly.mjs         |  2 +-
 tests/bootstrap.js        | 58 +++++++++++++++++++++++++++++----------
 tests/bootstrap_helper.js | 20 ++++++++++++++
 3 files changed, 65 insertions(+), 15 deletions(-)
 create mode 100644 tests/bootstrap_helper.js

diff --git a/tests/blockly.mjs b/tests/blockly.mjs
index cb836f539..e447cd3c2 100644
--- a/tests/blockly.mjs
+++ b/tests/blockly.mjs
@@ -27,7 +27,7 @@ if (window.BlocklyLoader) {
   // Uncompiled mode.  Use top-level await
   // (https://v8.dev/features/top-level-await) to block loading of
   // this module until goog.bootstrap()ping of Blockly is finished.
-  await window.BlocklyLoader;
+  await window.BlocklyLoader.done;
   Blockly = globalThis.Blockly;
 } else if (window.Blockly) {
   // Compiled mode.  Retrieve the pre-installed Blockly global.
diff --git a/tests/bootstrap.js b/tests/bootstrap.js
index 32003eecf..02cb145e0 100644
--- a/tests/bootstrap.js
+++ b/tests/bootstrap.js
@@ -131,21 +131,51 @@
               '</script>');
     }
 
-    const requiresString = options.requires.map(quote).join();
-    const scriptsString = options.additionalScripts.map(quote).join();
+    // Create a global variable to remember some stat that will be
+    // needed by later scripts.
+    window.BlocklyLoader = {
+      requires: options.requires,
+      done: null,
+    };
+
+    // Assemble a list of module targets to bootstrap.
+    //
+    // The first group of targets are those listed in
+    // options.requires.
+    //
+    // The next target is a fake one that will load
+    // bootstrap_helper.js.  For each we generate a call to
+    // goog.addDependency to tell the debug module loader that it can
+    // be loaded via a fake module name, and that it depends on all
+    // the targets in the first group (and indeed it will make a call
+    // to goog.require for each one).
+    //
+    // We then create another target for each of
+    // options.additionalScripts, again generating calls to
+    // goog.addDependency for each one making it dependent on the
+    // previous one.
+    let requires = options.requires.slice();
+    const scripts =
+        ['tests/bootstrap_helper.js'].concat(options.additionalScripts);
+    const scriptDeps = [];
+    for (let script, i = 0; script = scripts[i]; i++) {
+      const fakeModuleName = 'script.' + script.replace(/[./]/g, "-");
+      // requires.push(fakeModuleName);
+      scriptDeps.push('  goog.addDependency(' +
+          quote('../../' + script) + ', [' + quote(fakeModuleName) +
+          '], [' + requires.map(quote).join() + "], {'lang': 'es6'});\n");
+      requires = [fakeModuleName];
+    }
+
+    // Finally, write out a script containing the genrated
+    // goog.addDependency calls and a call to goog.bootstrap
+    // requesting the loading of the final target, which will cause
+    // all the previous ones to be loaded recursively.  Wrap this in a
+    // promise and save it so it can be awaited in blockly.mjs.
     document.write(
-        '<script>\n' +
-        '  let requires = [' + requiresString + '];\n' +
-        '  let scripts = [' + scriptsString + '];\n' +
-        '  for (const script of scripts) {\n' +
-        '    const fakeModuleName = \n' +
-        '        "script." + script.replace(/[./]/g, "-");\n' +
-        '    goog.addDependency("../../" + script, [fakeModuleName], \n' +
-        '        requires, {"lang": "es6"});\n' +
-        '    requires = [fakeModuleName];\n' +
-        '  }\n' +
-        '  window.BlocklyLoader = new Promise((resolve, reject) => {\n' +
-        '    goog.bootstrap(requires, resolve);\n' +
+        '<script>\n' + scriptDeps.join('') +
+        '  window.BlocklyLoader.done = new Promise((resolve, reject) => {\n' +
+        '    goog.bootstrap([' + requires.map(quote).join() + '], resolve);\n' +
         '  }).then(() => {\n' +
         '    return goog.module.get(\'Blockly\');\n' +
         '  });\n' +
diff --git a/tests/bootstrap_helper.js b/tests/bootstrap_helper.js
new file mode 100644
index 000000000..1bc4739b0
--- /dev/null
+++ b/tests/bootstrap_helper.js
@@ -0,0 +1,20 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * @fileoverview Helper script for bootstrap.js
+ *
+ * This is loaded, via goog.bootstrap(), after the other top-level
+ * Blockly modules.  It simply calls goog.require() for each of them,
+ * to force the deubg module loader to finish loading them before any
+ * non-module scripts (like msg/messages.js) that might have
+ * undeclared dependencies on them.
+ */
+
+/* eslint-disable-next-line no-undef */
+for (const require of BlocklyLoader.requires) {
+  goog.require(require);
+}