Merge pull request benjamn#50 from Gariest/DynamicImportOfflineBugFixv2

Enable retrying dynamic imports after failure.

Author: benjamn

install.js

@@ -143,17 +143,26 @@ makeInstaller = function (options) {
       var toBeFetched = missing;
       missing = null;
 
-      return Promise.resolve(
+      function clearPending() {
+        if (toBeFetched) {
+          Object.keys(toBeFetched).forEach(function (id) {
+            getOwn(filesByModuleId, id).pending = false;
+          });
+        }
+      }
+
+      return new Promise(function (resolve) {
         // The install.fetch function takes an object mapping missing
         // dynamic module identifiers to options objects, and should
         // return a Promise that resolves to a module tree that can be
         // installed. As an optimization, if there were no missing dynamic
         // modules, then we can skip calling install.fetch entirely.
-        toBeFetched && install.fetch(toBeFetched)
+        resolve(toBeFetched && install.fetch(toBeFetched));
 
-      ).then(function (tree) {
+      }).then(function (tree) {
         function both() {
           install(tree);
+          clearPending();
           return absChildId;
         }
 
@@ -165,6 +174,11 @@ makeInstaller = function (options) {
         // Whether previousPromise was resolved or rejected, carry on with
         // the installation regardless.
         return previousPromise.then(both, both);
+
+      }, function (error) {
+        // Fixes https://github.com/meteor/meteor/issues/10182.
+        clearPending();
+        throw error;
       });
     });
   };

test/run.js

@@ -981,6 +981,60 @@ describe("install", function () {
     });
   });
 
+  it("supports retrying dynamic imports after failure", function () {
+    var install = makeInstaller();
+
+    var threw = false;
+    install.fetch = function (ids) {
+      if (! threw) {
+        threw = true;
+        debugger;
+        throw new Error("network failure, or something");
+      }
+
+      var tree = {};
+
+      Object.keys(ids).forEach(function (id) {
+        var info = ids[id];
+        assert.strictEqual(info.module.id, id);
+        addToTree(tree, id, function (r, exports, module) {
+          assert.strictEqual(module, info.module);
+          exports.name = module.id;
+        });
+      });
+
+      return tree;
+    };
+
+    var require = install({
+      "main.js": function (require, exports, module) {
+        exports.attempt = function (id) {
+          return module.prefetch(id);
+        };
+      },
+      "a.js": ["./c", "./b"],
+      "b.js": ["./a", "./c"],
+      "c.js": ["./a", "./b"]
+    });
+
+    var attempt = require("./main").attempt;
+
+    return attempt("./a").then(function () {
+      throw new Error("should have failed");
+    }, function (error) {
+      assert.strictEqual(threw, true);
+      assert.strictEqual(
+        error.message,
+        "network failure, or something"
+      );
+
+      return attempt("./c").then(function (id) {
+        assert.strictEqual(id, "/c.js");
+        assert.strictEqual(require("./c").name, id);
+      });
+    });
+  });
+
   it("respects module.exports before file.contents", function () {
     var install = makeInstaller();