Require id in addRunDependency/removeRunDependency (#24890)

Since the dawn of time we've been emitting warnings when these are
called without an ID. It seem pretty reasonable to require one at this
point.

One of my motivations here it to update/replace this interface with a
more modern one based on promises. Making this API consistently use IDs
simplifies things. Also, even though this could be breaking change I
think its highly unlikely there are any users who are not specifying
IDs, since we have had warnings in debug builds since 2012:
f67ad60.

Author: sbc100

ChangeLog.md

@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
 
 4.0.13 (in development)
 -----------------------
+- The `addRunDependency`/`removeRunDependency` now assert in debug builds if
+  they are not passed an `id` parameter.  We have been issuing warnings in
+  this case since 2012 (f67ad60), so it seems highly unlikely anyone is not
+  passing IDs here. (#24890).
 - The `-sMODULARIZE` setting generates a factory function that must be called
   before the program is instantiated that run.  However, emscripten previously
   had very special case where this instantiation would happen automatically

src/preamble.js

@@ -251,32 +251,29 @@ function addRunDependency(id) {
 #endif
 
 #if ASSERTIONS
-  if (id) {
-    assert(!runDependencyTracking[id]);
-    runDependencyTracking[id] = 1;
-    if (runDependencyWatcher === null && typeof setInterval != 'undefined') {
-      // Check for missing dependencies every few seconds
-      runDependencyWatcher = setInterval(() => {
-        if (ABORT) {
-          clearInterval(runDependencyWatcher);
-          runDependencyWatcher = null;
-          return;
-        }
-        var shown = false;
-        for (var dep in runDependencyTracking) {
-          if (!shown) {
-            shown = true;
-            err('still waiting on run dependencies:');
-          }
-          err(`dependency: ${dep}`);
-        }
-        if (shown) {
-          err('(end of list)');
+  assert(id, 'addRunDependency requires an ID')
+  assert(!runDependencyTracking[id]);
+  runDependencyTracking[id] = 1;
+  if (runDependencyWatcher === null && typeof setInterval != 'undefined') {
+    // Check for missing dependencies every few seconds
+    runDependencyWatcher = setInterval(() => {
+      if (ABORT) {
+        clearInterval(runDependencyWatcher);
+        runDependencyWatcher = null;
+        return;
+      }
+      var shown = false;
+      for (var dep in runDependencyTracking) {
+        if (!shown) {
+          shown = true;
+          err('still waiting on run dependencies:');
         }
-      }, 10000);
-    }
-  } else {
-    err('warning: run dependency added without ID');
+        err(`dependency: ${dep}`);
+      }
+      if (shown) {
+        err('(end of list)');
+      }
+    }, 10000);
   }
 #endif
 }
@@ -289,12 +286,9 @@ function removeRunDependency(id) {
 #endif
 
 #if ASSERTIONS
-  if (id) {
-    assert(runDependencyTracking[id]);
-    delete runDependencyTracking[id];
-  } else {
-    err('warning: run dependency removed without ID');
-  }
+  assert(id, 'removeRunDependency requires an ID');
+  assert(runDependencyTracking[id]);
+  delete runDependencyTracking[id];
 #endif
   if (runDependencies == 0) {
 #if ASSERTIONS

test/code_size/test_codesize_hello_O0.json

@@ -1,10 +1,10 @@
 {
-  "a.out.js": 22324,
-  "a.out.js.gz": 8300,
+  "a.out.js": 22449,
+  "a.out.js.gz": 8326,
   "a.out.nodebug.wasm": 15143,
   "a.out.nodebug.wasm.gz": 7452,
-  "total": 37467,
-  "total_gz": 15752,
+  "total": 37592,
+  "total_gz": 15778,
   "sent": [
     "fd_write"
   ],

test/code_size/test_codesize_minimal_O0.json

@@ -1,10 +1,10 @@
 {
-  "a.out.js": 17587,
-  "a.out.js.gz": 6587,
+  "a.out.js": 17710,
+  "a.out.js.gz": 6612,
   "a.out.nodebug.wasm": 1136,
   "a.out.nodebug.wasm.gz": 659,
-  "total": 18723,
-  "total_gz": 7246,
+  "total": 18846,
+  "total_gz": 7271,
   "sent": [],
   "imports": [],
   "exports": [

test/code_size/test_unoptimized_code_size.json

@@ -1,16 +1,16 @@
 {
-  "hello_world.js": 53916,
-  "hello_world.js.gz": 17076,
+  "hello_world.js": 53804,
+  "hello_world.js.gz": 17065,
   "hello_world.wasm": 15143,
   "hello_world.wasm.gz": 7452,
   "no_asserts.js": 26714,
   "no_asserts.js.gz": 8952,
   "no_asserts.wasm": 12227,
   "no_asserts.wasm.gz": 6008,
-  "strict.js": 51966,
-  "strict.js.gz": 16409,
+  "strict.js": 51854,
+  "strict.js.gz": 16403,
   "strict.wasm": 15143,
   "strict.wasm.gz": 7438,
-  "total": 175109,
-  "total_gz": 63335
+  "total": 174885,
+  "total_gz": 63318
 }

test/other/codesize/test_codesize_minimal_O0.expected.js

@@ -508,44 +508,38 @@ var runDependencyWatcher = null;
 function addRunDependency(id) {
   runDependencies++;
 
-  if (id) {
-    assert(!runDependencyTracking[id]);
-    runDependencyTracking[id] = 1;
-    if (runDependencyWatcher === null && typeof setInterval != 'undefined') {
-      // Check for missing dependencies every few seconds
-      runDependencyWatcher = setInterval(() => {
-        if (ABORT) {
-          clearInterval(runDependencyWatcher);
-          runDependencyWatcher = null;
-          return;
-        }
-        var shown = false;
-        for (var dep in runDependencyTracking) {
-          if (!shown) {
-            shown = true;
-            err('still waiting on run dependencies:');
-          }
-          err(`dependency: ${dep}`);
-        }
-        if (shown) {
-          err('(end of list)');
+  assert(id, 'addRunDependency requires an ID')
+  assert(!runDependencyTracking[id]);
+  runDependencyTracking[id] = 1;
+  if (runDependencyWatcher === null && typeof setInterval != 'undefined') {
+    // Check for missing dependencies every few seconds
+    runDependencyWatcher = setInterval(() => {
+      if (ABORT) {
+        clearInterval(runDependencyWatcher);
+        runDependencyWatcher = null;
+        return;
+      }
+      var shown = false;
+      for (var dep in runDependencyTracking) {
+        if (!shown) {
+          shown = true;
+          err('still waiting on run dependencies:');
         }
-      }, 10000);
-    }
-  } else {
-    err('warning: run dependency added without ID');
+        err(`dependency: ${dep}`);
+      }
+      if (shown) {
+        err('(end of list)');
+      }
+    }, 10000);
   }
 }
 
 function removeRunDependency(id) {
   runDependencies--;
 
-  if (id) {
-    assert(runDependencyTracking[id]);
-    delete runDependencyTracking[id];
-  } else {
-    err('warning: run dependency removed without ID');
-  }
+  assert(id, 'removeRunDependency requires an ID');
+  assert(runDependencyTracking[id]);
+  delete runDependencyTracking[id];
   if (runDependencies == 0) {
     if (runDependencyWatcher !== null) {
       clearInterval(runDependencyWatcher);

test/test_browser.py

@@ -2379,11 +2379,11 @@ def test_pre_run_deps(self):
     # Adding a dependency in preRun will delay run
     create_file('pre.js', '''
       Module.preRun = () => {
-        addRunDependency();
+        addRunDependency('foo');
         out('preRun called, added a dependency...');
         setTimeout(function() {
           Module.okk = 10;
-          removeRunDependency()
+          removeRunDependency('foo')
         }, 2000);
       };
     ''')