Merge pull request #1358 from stealjs/lr-failed

When a module is deleted, ensure any 'failed' records are removed
stealjs · Mar 14, 2018 · 2093b92 · 2093b92
2 parents cdbf99f + fb22fcd
commit 2093b92
Show file tree

Hide file tree

Showing 6 changed files with 137 additions and 24 deletions.
diff --git a/ext/live-reload.js b/ext/live-reload.js
@@ -114,6 +114,9 @@ function addLiveReload(loader) {
 				delete loader._liveListeners[moduleName];
 			}
 			return true;
+		} else {
+			// Delete it anyways to destroy extra state.
+			loader.delete(moduleName);
 		}
 		return false;
 	}

diff --git a/src/loader/lib/loader.js b/src/loader/lib/loader.js
@@ -1006,6 +1006,16 @@ function logloads(loads) {
       var loader = this._loader;
       delete loader.importPromises[name];
       delete loader.moduleRecords[name];
+	  if(this.failed) {
+		  var load;
+		  for(var i = 0; i < this.failed.length; i++) {
+			  load = this.failed[i];
+			  if(load.name === name) {
+				  this.failed.splice(i, 1);
+				  break;
+			  }
+		  }
+	  }
       return loader.modules[name] ? delete loader.modules[name] : false;
     },
     // 26.3.3.4 entries not implemented

diff --git a/src/loader/loader-sans-promises.js b/src/loader/loader-sans-promises.js
@@ -1042,6 +1042,16 @@ function logloads(loads) {
       var loader = this._loader;
       delete loader.importPromises[name];
       delete loader.moduleRecords[name];
+	  if(this.failed) {
+		  var load;
+		  for(var i = 0; i < this.failed.length; i++) {
+			  load = this.failed[i];
+			  if(load.name === name) {
+				  this.failed.splice(i, 1);
+				  break;
+			  }
+		  }
+	  }
       return loader.modules[name] ? delete loader.modules[name] : false;
     },
     // 26.3.3.4 entries not implemented

diff --git a/src/loader/loader.js b/src/loader/loader.js
@@ -2312,6 +2312,16 @@ function logloads(loads) {
       var loader = this._loader;
       delete loader.importPromises[name];
       delete loader.moduleRecords[name];
+	  if(this.failed) {
+		  var load;
+		  for(var i = 0; i < this.failed.length; i++) {
+			  load = this.failed[i];
+			  if(load.name === name) {
+				  this.failed.splice(i, 1);
+				  break;
+			  }
+		  }
+	  }
       return loader.modules[name] ? delete loader.modules[name] : false;
     },
     // 26.3.3.4 entries not implemented

diff --git a/steal.js b/steal.js
@@ -2312,6 +2312,16 @@ function logloads(loads) {
       var loader = this._loader;
       delete loader.importPromises[name];
       delete loader.moduleRecords[name];
+	  if(this.failed) {
+		  var load;
+		  for(var i = 0; i < this.failed.length; i++) {
+			  load = this.failed[i];
+			  if(load.name === name) {
+				  this.failed.splice(i, 1);
+				  break;
+			  }
+		  }
+	  }
       return loader.modules[name] ? delete loader.modules[name] : false;
     },
     // 26.3.3.4 entries not implemented

diff --git a/test/live_reload/unit.js b/test/live_reload/unit.js
@@ -2,19 +2,36 @@ var reloader = require("live-reload");
 var loader = require("@loader");
 loader.liveReloadReload = false;
 
+function hookFetch(fn) {
+	var oldFetch = loader.fetch;
+	var defaultFetch = function(self, args){
+		return function(){
+			return oldFetch.apply(self, args);
+		};
+	}
+	loader.fetch = function(){
+		var args = [defaultFetch(this, arguments)].concat([].slice.call(arguments));
+		return fn.apply(this, args);
+	}
+	return function(){
+		loader.fetch = oldFetch;
+	};
+}
+
 QUnit.module("Unit tests", {
 	beforeEach: function(){
-		var fetch = this.oldFetch = loader.fetch;
-		loader.fetch = function(load){
+		this.undoHook = hookFetch(function(fetch, load){
 			if(load.name === "foo") {
 				return Promise.resolve("exports.foo = 'bar'");
 			} else if(load.name === "bar") {
 				return Promise.resolve("exports.bar = 'bam'");
+			} else {
+				debugger;
 			}
-		};
+		});
 	},
 	afterEach: function(){
-		loader.fetch = this.oldFetch;
+		this.undoHook();
 		loader.delete("foo");
 		loader.delete("bar");
 	}
@@ -91,27 +108,29 @@ QUnit.test("Returns an error when there is an error", function(assert){
 	var done = assert.async();
 	var loaded = false;
 
-	var fetch = loader.fetch;
-	loader.fetch = function(load){
+	var undo = hookFetch(function(fetch, load){
 		if(load.name === "oops") {
 			if(loaded) {
 				return Promise.resolve("module.exports = {}; oops 'bad';");
 			} else {
 				return Promise.resolve("module.exports = {};");
 			}
+		} else {
+			return fetch();
 		}
-		return fetch.apply(this, arguments);
-	};
+	});
 
 	return loader.import("oops")
 	.then(function(){
 		loaded = true;
+		loader.set("error-in-tree", loader.newModule({}));
 		return loader.import("live-reload", { name: "error-in-tree" })
 	})
 	.then(function(reload){
 		reload(function(err){
 			assert.ok(err instanceof Error, "Got an error");
-			loader.fetch = fetch;
+			reload.disposeModule("error-in-tree");
+			undo();
 			done();
 		});
 
@@ -153,16 +172,16 @@ QUnit.test("are deleted when a parent module no longer uses it", function(assert
 	var done = assert.async();
 	var fooSrc;
 
-	var fetch = loader.fetch;
-	var test = this;
-	loader.fetch = function(load){
+	var undo = hookFetch(function(fetch, load){
 		if(load.name === "foo") {
 			var src = fooSrc || "req" + "uire('bar');";
 			return Promise.resolve(src);
 		} else if(load.name === "bar") {
 			return Promise.resolve("exports.foo = 'bar';");
+		} else {
+			return fetch();
 		}
-	};
+	});
 
 	loader.import("foo")
 	.then(function(){
@@ -177,7 +196,7 @@ QUnit.test("are deleted when a parent module no longer uses it", function(assert
 		assert.ok(!loader.has("bar"), "orphaned module removed");
 	})
 	.then(function(){
-		loader.fetch = fetch;
+		undo();
 		loader.delete("foo");
 		loader.delete("bar");
 	})
@@ -186,20 +205,21 @@ QUnit.test("are deleted when a parent module no longer uses it", function(assert
 
 QUnit.test("are not removed if they have another parent", function(assert){
 	var done = assert.async();
+	var test = this;
 	var fooSrc;
 
-	var fetch = loader.fetch;
-	var test = this;
-	loader.fetch = function(load){
+	var undo = hookFetch(function(fetch, load){
 		if(load.name === "foo") {
 			var src = fooSrc || "req" + "uire('bar');";
 			return Promise.resolve(src);
 		} else if(load.name === "bar") {
 			return Promise.resolve("exports.foo = 'bar';");
 		} else if(load.name === "qux") {
 			return Promise.resolve("req" + "uire('bar');");
+		} else {
+			return fetch();
 		}
-	};
+	});
 
 	loader.import("foo")
 	.then(function(){
@@ -217,31 +237,36 @@ QUnit.test("are not removed if they have another parent", function(assert){
 		assert.ok(loader.has("bar"), "orphaned module removed");
 	})
 	.then(function(){
-		loader.fetch = fetch;
+		undo();
 		loader.delete("foo");
 		loader.delete("bar");
 		loader.delete("qux");
 	})
-	.then(done, done);
+	.then(done, function(err){
+		undo();
+		assert.notOk(err);
+		done(err);
+	});
 });
 
 QUnit.module("reload.dispose", {
 	beforeEach: function(assert){
 		var test = this;
 
-		var fetch = this.oldFetch = loader.fetch;
-		loader.fetch = function(load){
+		this.undoHook = hookFetch(function(fetch, load){
 			if(load.name === "foo") {
 				return Promise.resolve("requ" + "ire('bar');");
 			} else if(load.name === "bar") {
 				return Promise.resolve("exports.bar = 'bam'");
 			} else if(load.name === "baz") {
 				return Promise.resolve("requ" + "ire('foo');");
+			} else {
+				return fetch();
 			}
-		};
+		});
 	},
 	afterEach: function(){
-		loader.fetch = this.oldFetch;
+		this.undoHook();
 		loader.delete("foo");
 		loader.delete("bar");
 	}
@@ -317,4 +342,49 @@ QUnit.test("Emits a dispose event for a module that is removed", function(assert
 	.then(done, done);
 });
 
+QUnit.test("Removes modules that have previously failed", function(assert){
+	var done = assert.async();
+
+	var undo = hookFetch(function(fetch, load){
+		if(load.name === "some-parent-module") {
+			return Promise.resolve("import 'some-module';");
+		} else if(load.name === "some-module") {
+			return Promise.resolve("export default {}; import oops();")
+		} else {
+			return fetch();
+		}
+	});
+
+	loader.import("some-parent-module")
+	.then(null, function(err){
+		undo();
+		return loader.import("live-reload", { name: "dispose-failed-test" });
+	})
+	.then(function(reload){
+		reload.disposeModule("some-module");
+
+		undo = hookFetch(function(fetch, load){
+			if(load.name === "some-parent-module") {
+				return Promise.resolve("import 'some-module';");
+			} else if(load.name === "some-module") {
+				return Promise.resolve("export default {};");
+			} else {
+				return fetch();
+			}
+		});
+
+		// Should work now
+		return loader.import("some-parent-module");
+	})
+	.then(function(v){
+		undo();
+		assert.ok(true, "Finished without error");
+		done();
+	}, function(err){
+		undo();
+		assert.notOk(err, "Failed to load once disposed");
+		done(err);
+	});
+});
+
 QUnit.start();