Debug loader returns `null` when loading an ES module if `goog.bootstrap` called from a `<script type=module>` #1152

cpcallen · 2022-02-11T17:01:04Z

Consider an ES module using declareModuleId:

goog.declareModuleId('esModule');
export const foo = 'foo';

which is required by a goog.module:

goog.module('googModule');
const {foo} = goog.require('esModule');

If this is loaded as follows:

<html>
  <head>
    <script src="../node_modules/google-closure-library/closure/goog/base.js"></script>
    <script src="deps.js"></script>
    <script>
      goog.bootstrap(['googModule']);
    </script>
  </head>
</html>

then all is well.

If, however, the final <script> tag is changed to a <script type=module>, then this fails with

Uncaught TypeError: Cannot destructure property 'foo' of 'goog.require(...)' as it is null.

…and indeed, this is because goog.DebugLoader_.prototype.load_ has returned null; this appears to be because the goog.require call in the dependant module has happened before its dependency has finished loading.

This problem does not occur if the dependant module is converted to also be an ES module:

goog.declareModuleId('googModule');
const {foo} = goog.require('esModule');
export const bar = 'bar';

The text was updated successfully, but these errors were encountered:

Move the calls to goog.bootstrap from blockly.mjs to prepare.mjs. This is needed to work around a bug in the Cosure Library debug loader (google/closure-library#1152). This gets a bit ugly because most of the code has to go in a <script> (because it needs goog.bootstrap, which was loaded by an earlier <script> tag).

…playground (#5931) * chore(deps): Update closure/goog/base.js, add goog.js * Update base.js from the latest version (20220104.0.0). * Also copy over goog.js, which provides access to asuitable subset of goog.* via an importable module). * refactor(tests): Have playground.html load Blockly as a module N.B.: * We still need a preparation step, in order to load base.js and deps.js via <script> tags in uncompiled mode; in compiled mode it will instead load all the *_compressed.js files via <script> tags. Acess to the Blockly object is via: import Blockly from './playgrounds/blockly.mjs'; (N.B: no "* as", since blockly.mjs has only a default export.) * There remain two serious defects when running in uncompiled mode: * It does not attempt to load msg/messages.js, causing startup to fail. * Module loading only works if there are no ES Modules; if there are, something goes wrong with base.js's attempt to sequence module loads causing goog.modules that import ES modules to get a null exports object for that import. X-( * fix(tests): Have playground.html load messages.js before generators This fixes the issue caused by missing messages when loading the generators. * fix(tests): Move bootsrap calls to prepare.js Move the calls to goog.bootstrap from blockly.mjs to prepare.mjs. This is needed to work around a bug in the Cosure Library debug loader (google/closure-library#1152). This gets a bit ugly because most of the code has to go in a <script> (because it needs goog.bootstrap, which was loaded by an earlier <script> tag). * fix(documentation): Minor comment corrections for PR #5931

shicks · 2022-02-25T23:35:25Z

High level response: the person who wrote all this code left Google a while back, and we don't really use ES modules internally at the moment, so this is pretty poorly covered. Is there a reason the goog.bootstrap script needs to be a module? It sounds like the easiest answer is just "don't do that" - only call goog.bootstrap from traditional (non-module) scripts.

Going deeper - I'm stepping through the debug loader and it looks like the difference is that when it's loading the googModule dependency, goog.Dependency.defer_ is only true when the bootstrap is a script. For some reason, defer_ is false when called from a module, and thus, needsAsyncLoading ends up false as well, and the loader tries to load googModule synchronously after writing the script tag for esModule, but before it's actually loaded.

I've got to step away for a bit, but the next step would be to figure out what's setting defer_ to true in the one case but failing to in the other.

shicks · 2022-02-26T06:02:06Z

When the esModule is written, there's a check for goog.isDocumentLoading_(), and it only sets defer_ = true (and uses document.write) if it's currently loading. When the bootstrap is in a module, the document's no longer loading, so it uses append instead of write and doesn't set defer. But it's appending a module and the non-deferred googModule load is appending a non-module, so it ends up executing first.

I tried to fix this by also checking if there are any ES modules pending and then setting defer_ to true in that case, This works for Chrome, but seems to cause a subtle heisenbug in Firefox: normally when loading ES modules, the debug loader adds 4 script (module) tags: a "before" script to set the expectation that an ES module will be loaded, a script with src= to load the file, a script to import and pass the module's exports object back to Closure, and finally an "after" script to clear the ES module expectation. When not debugging in FF, the "after" module loads before the actual loaded module, so that the loader is no longer expecting an ES module to show up.

This seems like a fundamental issue with Firefox. The rationale behind using a separate "after" script is that if the loaded module fails to load, we still want to reset the state, so we really do depend on the browser getting the order right here. Here's a minimal repro to see the problematic Firefox behavior:

// file: a.js
console.log('inside');

<script type="module">
function s(src, body) {
  const script = document.createElement('script');
  if (src) script.src = src;
  if (body) script.innerHTML = body;
  script.type = 'module';
  script.defer = true;
  document.head.appendChild(script);
}

s('', `console.log('before');`);
s('a.js');
s('', `console.log('after');`);
</script>

In Chrome, this logs (before, inside, after), while in Firefox, it logs (before, after, inside). I suspect this is a Firefox bug? I don't understand why this works when the bootstrap command is not in a module (the only difference seems to be the defer attribute on the appended script tags - but in my experiment, removing defer didn't fix it, nor did even making the main script a non-module). Given this, I'm at a loss for how to fix it.

shicks · 2022-02-26T06:17:59Z

Okay, and now that repro is also printing (before, after, inside) in Chrome. So now I really have no idea what's going on. I wonder if this use case is just fundamentally broken?

cpcallen · 2022-02-28T18:23:58Z

High level response: the person who wrote all this code left Google a while back, and we don't really use ES modules internally at the moment, so this is pretty poorly covered.

Understood—though I was mildly surprised that this hadn't been noticed by other teams doing a goog.module -> ES Module -> TS conversion like we are.

Is there a reason the goog.bootstrap script needs to be a module? It sounds like the easiest answer is just "don't do that" - only call goog.bootstrap from traditional (non-module) scripts.

We have an app that was basically a single .html` page:

It originally used <script src=/*...*/> tags to load all of our goog.provides modules.
We then modified it to use <script src to load base.js and (our) deps.js, then do a <script>goog.require(/*...*/)</script> to load our goog.modules.
We then started converting some of those module to ES Modules and found that they (and every goog.module depending on them) wouldn't finish loading until after the <body onload=/*...*/> event handler that attempted (and now failed) to make calls into the loaded modules had run.
After a lot of poking around I eventually discovered goog.bootstrap, and in seeking to figure out how to get code in <script> tags in our .html file to wait for the bootstrap callback to load, eventually came up with (simplified):

<!-- index.html -->
<script src="closure/goog/base.js"></script>
<script src="build/deps.js"></script>
<script type=module>
  import myModule from 'wrapper.mjs';
  myModule.doSomething();
</script>

// wrapper.mjs
await new Promise((resolve, reject) => {
  goog.bootstrap([/* modules to bootstrap */], resolve)
});
// N.B. top-level module still contains a goog.module.declareLegacyNamespace.
export default globalThis.myModule;

which was nice because it kept the code in index.html file very clean, hiding the crufty bits in wrapper.js

Unfortunately as noted this did not work as expected, so for now I have changed it to:

<!-- index.html -->
<script src="closure/goog/base.js"></script>
<script src="build/deps.js"></script>
<script>
  window.loaded = new Promise((resolve, reject) => {
    goog.bootstrap([/* modules to bootstrap */], resolve)
  });
</script>
<script type=module>
  await window.loaded;
  myModule.doSomething();
</script>

which feels a little kludgy but works fine.

Thanks for looking in to this issue. It sounds like it may not be easy to fix properly, but I hope that at least having a report open will help anyone else who encounters it.

shicks · 2022-03-01T23:38:47Z

I see. Thanks for reporting this, and expanding on what you're doing.

Unfortunately, goog.bootstrap is pretty unmaintained, as you may have gathered from the scant documentation. The short story is that we tried to pivot to ES modules before ultimately settling on TypeScript, and someone on the team at the time rewrote the debug loader (including coming up with goog.bootstrap) to handle the combinatorial explosion of dependency types (goog.provide, goog.module, ES modules) and the various interdependencies between them. But they left the company years ago and now nobody really understands the debug loader all the well. With our various renewed efforts to get more users onto bundlers and development servers, we're mostly trying to turn down the debug loader. Internally, our TypeScript stack doesn't use ES modules, so our well-lit migration path doesn't involve migrating goog.modules to ES modules. (Unfortunately, if you need to build against opensource, there's not much we can do).

Your workaround is indeed kludgy, and I'm sorry that that's what it takes, but as you've gathered, it's unlikely we'll be able to fix this "the right way". If anybody can figure out how to adjust the debug loader to make it work in all ES-module-supporting browsers, we'd welcome a contribution, but it's not something we're going to be able to prioritize.

shicks added the P3 - PRs Only label Mar 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Debug loader returns `null` when loading an ES module if `goog.bootstrap` called from a `<script type=module>` #1152

Debug loader returns `null` when loading an ES module if `goog.bootstrap` called from a `<script type=module>` #1152

cpcallen commented Feb 11, 2022

shicks commented Feb 25, 2022

shicks commented Feb 26, 2022

shicks commented Feb 26, 2022

cpcallen commented Feb 28, 2022

shicks commented Mar 1, 2022

Debug loader returns null when loading an ES module if goog.bootstrap called from a <script type=module> #1152

Debug loader returns null when loading an ES module if goog.bootstrap called from a <script type=module> #1152

Comments

cpcallen commented Feb 11, 2022

shicks commented Feb 25, 2022

shicks commented Feb 26, 2022

shicks commented Feb 26, 2022

cpcallen commented Feb 28, 2022

shicks commented Mar 1, 2022

Debug loader returns `null` when loading an ES module if `goog.bootstrap` called from a `<script type=module>` #1152

Debug loader returns `null` when loading an ES module if `goog.bootstrap` called from a `<script type=module>` #1152