Merge pull request #13276 from webpack/bugfix/emit-caching

emit assets even when they were cleaned from fs in the meantime

Author: sokra

lib/Compiler.js

@@ -263,6 +263,8 @@ class Compiler {
 		this._assetEmittingSourceCache = new WeakMap();
 		/** @private @type {Map<string, number>} */
 		this._assetEmittingWrittenFiles = new Map();
+		/** @private @type {Set<string>} */
+		this._assetEmittingPreviousFiles = new Set();
 	}
 
 	/**
@@ -556,6 +558,8 @@ class Compiler {
 			compilation.assets = { ...compilation.assets };
 			/** @type {Map<string, { path: string, source: Source, size: number, waiting: { cacheEntry: any, file: string }[] }>} */
 			const caseInsensitiveMap = new Map();
+			/** @type {Set<string>} */
+			const allTargetPaths = new Set();
 			asyncLib.forEachLimit(
 				assets,
 				15,
@@ -583,6 +587,7 @@ class Compiler {
 							outputPath,
 							targetFile
 						);
+						allTargetPaths.add(targetPath);
 
 						// check if the target file has already been written by this Compiler
 						const targetFileGeneration = this._assetEmittingWrittenFiles.get(
@@ -775,18 +780,22 @@ ${other}`);
 							// check if the Source has been written to this target file
 							const writtenGeneration = cacheEntry.writtenTo.get(targetPath);
 							if (writtenGeneration === targetFileGeneration) {
-								// if yes, we skip writing the file
-								// as it's already there
-								// (we assume one doesn't remove files while the Compiler is running)
+								// if yes, we may skip writing the file
+								// if it's already there
+								// (we assume one doesn't modify files while the Compiler is running, other then removing them)
 
-								compilation.updateAsset(file, cacheEntry.sizeOnlySource, {
-									size: cacheEntry.sizeOnlySource.size()
-								});
-
-								return callback();
-							}
+								if (this._assetEmittingPreviousFiles.has(targetPath)) {
+									// We assume that assets from the last compilation say intact on disk (they are not removed)
+									compilation.updateAsset(file, cacheEntry.sizeOnlySource, {
+										size: cacheEntry.sizeOnlySource.size()
+									});
 
-							if (!immutable) {
+									return callback();
+								} else {
+									// Settings immutable will make it accept file content without comparing when file exist
+									immutable = true;
+								}
+							} else if (!immutable) {
 								if (checkSimilarFile()) return;
 								// We wrote to this file before which has very likely a different content
 								// skip comparing and assume content is different for performance
@@ -822,7 +831,12 @@ ${other}`);
 				err => {
 					// Clear map to free up memory
 					caseInsensitiveMap.clear();
-					if (err) return callback(err);
+					if (err) {
+						this._assetEmittingPreviousFiles.clear();
+						return callback(err);
+					}
+
+					this._assetEmittingPreviousFiles = allTargetPaths;
 
 					this.hooks.afterEmit.callAsync(compilation, err => {
 						if (err) return callback(err);

test/watchCases/cache/asset-modules/0/index.js

@@ -1,11 +1,16 @@
-it("should return a valid url when cached", () => {
+import { stat } from "fs";
+import { promisify } from "util";
+
+it("should return a valid url when cached", async () => {
 	const url = new URL("file.txt", import.meta.url);
 	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
 });
 
-it("should return a valid url when modified", () => {
+it("should return a valid url when modified", async () => {
 	const url = new URL("other.txt", import.meta.url);
 	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
 });
 
 it("should not emit undefined files", () => {

test/watchCases/cache/asset-modules/2/index.js

@@ -0,0 +1,5 @@
+it("should not emit files", () => {
+	expect(STATS_JSON.assets.map(a => a.name)).not.toContainEqual(
+		expect.stringMatching(/\.txt$/)
+	);
+});

test/watchCases/cache/asset-modules/3/index.js

@@ -0,0 +1,19 @@
+import { stat } from "fs";
+import { promisify } from "util";
+
+it("should return a valid url when cached", async () => {
+	const url = new URL("file.txt", import.meta.url);
+	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
+});
+
+it("should return a valid url when modified", async () => {
+	const url = new URL("other.txt", import.meta.url);
+	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
+});
+
+it("should not emit undefined files", () => {
+	expect(STATS_JSON.assets.map(a => a.name)).not.toContain(undefined);
+	expect(STATS_JSON.assets.map(a => a.name)).not.toContain("undefined");
+});

test/watchCases/cache/emit-when-clean/0/file.txt

@@ -0,0 +1 @@
+Hello World

test/watchCases/cache/emit-when-clean/0/index.js

@@ -0,0 +1,14 @@
+import { stat } from "fs";
+import { promisify } from "util";
+
+it("should return a valid url when cached", async () => {
+	const url = new URL("file.txt", import.meta.url);
+	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
+});
+
+it("should return a valid url when modified", async () => {
+	const url = new URL("other.txt", import.meta.url);
+	expect(url.pathname).toMatch(/\.txt$/);
+	expect((await promisify(stat)(url)).isFile()).toBe(true);
+});

test/watchCases/cache/emit-when-clean/0/other.txt

@@ -0,0 +1 @@
+Hello World 2

test/watchCases/cache/emit-when-clean/1/file.txt

@@ -0,0 +1 @@
+Hello World

test/watchCases/cache/emit-when-clean/1/other.txt

@@ -0,0 +1 @@
+Hello World 3

test/watchCases/cache/emit-when-clean/2/index.js

@@ -0,0 +1,5 @@
+it("should not emit files", () => {
+	expect(STATS_JSON.assets.map(a => a.name)).not.toContainEqual(
+		expect.stringMatching(/\.txt$/)
+	);
+});