use - instead of ~ as default for better compat

sokra · sokra · commit fe80339d7163 · 2018-12-07T14:46:49.000+01:00
fix automaticNameDelimiter in splitChunks
fix test cases
diff --git a/lib/WebpackOptionsDefaulter.js b/lib/WebpackOptionsDefaulter.js
@@ -267,7 +267,7 @@ class WebpackOptionsDefaulter extends OptionsDefaulter {
 		this.set("optimization.splitChunks.maxAsyncRequests", "make", options => {
 			return isProductionLikeMode(options) ? 6 : Infinity;
 		});
-		this.set("optimization.splitChunks.automaticNameDelimiter", "~");
+		this.set("optimization.splitChunks.automaticNameDelimiter", "-");
 		this.set("optimization.splitChunks.maxInitialRequests", "make", options => {
 			return isProductionLikeMode(options) ? 4 : Infinity;
 		});
diff --git a/lib/ids/NamedChunkIdsPlugin.js b/lib/ids/NamedChunkIdsPlugin.js
@@ -21,7 +21,7 @@ const {
 
 class NamedChunkIdsPlugin {
 	constructor(options) {
-		this.delimiter = (options && options.delimiter) || "~";
+		this.delimiter = (options && options.delimiter) || "-";
 		this.context = options && options.context;
 	}
 
diff --git a/lib/optimize/SplitChunksPlugin.js b/lib/optimize/SplitChunksPlugin.js
@@ -288,6 +288,7 @@ module.exports = class SplitChunksPlugin {
 							maxInitialRequests: option.maxInitialRequests,
 							filename: option.filename,
 							idHint: option.idHint,
+							automaticNameDelimiter: option.automaticNameDelimiter,
 							reuseExistingChunk: option.reuseExistingChunk
 						});
 					}
@@ -748,13 +749,13 @@ module.exports = class SplitChunksPlugin {
 						});
 
 						// Skip when no chunk selected
-						if (usedChunks.length === 0) continue;
+						if (usedChunks.length === 0 && !isReused) continue;
 
 						if (
 							Number.isFinite(item.cacheGroup.maxInitialRequests) ||
 							Number.isFinite(item.cacheGroup.maxAsyncRequests)
 						) {
-							const chunkInLimit = usedChunks.filter(chunk => {
+							const chunksInLimit = usedChunks.filter(chunk => {
 								// respect max requests when not enforced
 								const maxRequests = chunk.isOnlyInitial()
 									? item.cacheGroup.maxInitialRequests
@@ -769,13 +770,15 @@ module.exports = class SplitChunksPlugin {
 								);
 							});
 
-							if (chunkInLimit.length < usedChunks.length) {
-								if (chunkInLimit.length >= item.cacheGroup.minChunks) {
+							if (isReused) chunksInLimit.push(newChunk);
+
+							if (chunksInLimit.length < usedChunks.length) {
+								if (chunksInLimit.length >= item.cacheGroup.minChunks) {
 									for (const module of item.modules) {
 										addModuleToChunksInfoMap(
 											item.cacheGroup,
-											chunkInLimit,
-											getKey(chunkInLimit),
+											chunksInLimit,
+											getKey(chunksInLimit),
 											module
 										);
 									}
diff --git a/test/__snapshots__/ConfigTestCases.test.js.snap b/test/__snapshots__/ConfigTestCases.test.js.snap
@@ -4,26 +4,26 @@ exports[`ConfigTestCases records issue-2991 exported tests should write relative
 "{
   \\"chunks\\": {
     \\"byName\\": {
-      \\"main\\": 0
+      \\"main\\": 404
     },
     \\"bySource\\": {
     },
     \\"usedIds\\": [
-      0
+      404
     ]
   },
   \\"modules\\": {
     \\"byIdentifier\\": {
-      \\"external \\\\\\"fs\\\\\\"\\": 188,
-      \\"external \\\\\\"path\\\\\\"\\": 158,
-      \\"ignored  pkgs/somepackage/foo\\": 569,
+      \\"external \\\\\\"fs\\\\\\"\\": 66,
+      \\"external \\\\\\"path\\\\\\"\\": 589,
+      \\"ignored  pkgs/somepackage/foo\\": 760,
       \\"test.js\\": 316
     },
     \\"usedIds\\": [
-      158,
-      188,
+      66,
       316,
-      569
+      589,
+      760
     ]
   }
 }"
@@ -33,30 +33,30 @@ exports[`ConfigTestCases records issue-7339 exported tests should write relative
 "{
   \\"chunks\\": {
     \\"byName\\": {
-      \\"main\\": 0
+      \\"main\\": 404
     },
     \\"bySource\\": {
     },
     \\"usedIds\\": [
-      0
+      404
     ]
   },
   \\"modules\\": {
     \\"byIdentifier\\": {
-      \\"dependencies sync /^\\\\\\\\.\\\\\\\\/.*$/\\": 913,
+      \\"dependencies sync /^\\\\\\\\.\\\\\\\\/.*$/\\": 154,
       \\"dependencies/bar.js\\": 853,
       \\"dependencies/foo.js\\": 494,
-      \\"external \\\\\\"fs\\\\\\"\\": 188,
-      \\"external \\\\\\"path\\\\\\"\\": 158,
+      \\"external \\\\\\"fs\\\\\\"\\": 66,
+      \\"external \\\\\\"path\\\\\\"\\": 589,
       \\"test.js\\": 316
     },
     \\"usedIds\\": [
-      158,
-      188,
+      66,
+      154,
       316,
       494,
-      853,
-      913
+      589,
+      853
     ]
   }
 }"
diff --git a/test/__snapshots__/StatsTestCases.test.js.snap b/test/__snapshots__/StatsTestCases.test.js.snap
diff --git a/test/configCases/async-commons-chunk/existing-name/webpack.config.js b/test/configCases/async-commons-chunk/existing-name/webpack.config.js
@@ -4,8 +4,7 @@ module.exports = {
 	},
 	optimization: {
 		splitChunks: {
-			minSize: 1,
-			name: true
+			minSize: 1
 		},
 		chunkIds: "named"
 	}
diff --git a/test/configCases/hash-length/output-filename/test.config.js b/test/configCases/hash-length/output-filename/test.config.js
@@ -39,7 +39,7 @@ module.exports = {
 					}, files: ${files.join(", ")})`
 				);
 			}
-			verifyFilenameLength(filename, bundleDetect.expectedNameLength);
+			verifyFilenameLength(filename.replace(/^\d+\./, "X."), bundleDetect.expectedNameLength);
 		}
 
 		return "./" + filename;
diff --git a/test/configCases/split-chunks/chunk-filename-delimiter-default/index.js b/test/configCases/split-chunks/chunk-filename-delimiter-default/index.js
@@ -8,7 +8,6 @@ it("should run", function() {
 	);
 
 	const files = require("fs").readdirSync(__dirname);
-	const hasFile = files.indexOf('a~b~c.bundle.js') !== -1;
-
-	expect(hasFile).toBe(true);
+	expect(files).toContain('a-a_js-2a91f0ff.bundle.js');
+	expect(files).toContain('b-b_js-c441f481.bundle.js');
 });
diff --git a/test/configCases/split-chunks/chunk-filename-delimiter-default/webpack.config.js b/test/configCases/split-chunks/chunk-filename-delimiter-default/webpack.config.js
@@ -1,4 +1,5 @@
 module.exports = {
+	mode: "development",
 	entry: {
 		main: "./index"
 	},
@@ -13,7 +14,14 @@ module.exports = {
 	},
 	optimization: {
 		splitChunks: {
-			minSize: 1
+			cacheGroups: {
+				async: {
+					chunks: "async",
+					reuseExistingChunk: true,
+					minSize: 1,
+					maxSize: 1
+				}
+			}
 		}
 	}
 };
diff --git a/test/configCases/split-chunks/chunk-filename-delimiter/b.js b/test/configCases/split-chunks/chunk-filename-delimiter/b.js
@@ -1,3 +1,4 @@
 const c = require("./commons");
+require("./c");
 
 module.exports = "b" + c;
diff --git a/test/configCases/split-chunks/chunk-filename-delimiter/index.js b/test/configCases/split-chunks/chunk-filename-delimiter/index.js
@@ -2,13 +2,11 @@ it("should run", function() {
 	Promise.all(
 		[
 			import(/* webpackChunkName: "a" */ "./a"),
-			import(/* webpackChunkName: "b" */ "./b"),
-			import(/* webpackChunkName: "c" */ "./c")
+			import(/* webpackChunkName: "b" */ "./b")
 		]
 	);
 
 	const files = require("fs").readdirSync(__dirname);
-	const hasFile = files.indexOf('a-b-c.bundle.js') !== -1;
-
-	expect(hasFile).toBe(true);
+	expect(files).toContain('a---a_js---2a91f0ff.bundle.js');
+	expect(files).toContain('b---b_js---c441f481.bundle.js');
 });
diff --git a/test/configCases/split-chunks/chunk-filename-delimiter/webpack.config.js b/test/configCases/split-chunks/chunk-filename-delimiter/webpack.config.js
@@ -1,4 +1,5 @@
 module.exports = {
+	mode: "development",
 	entry: {
 		main: "./index"
 	},
@@ -13,8 +14,15 @@ module.exports = {
 	},
 	optimization: {
 		splitChunks: {
-			automaticNameDelimiter: "-",
-			minSize: 1
+			cacheGroups: {
+				async: {
+					chunks: "async",
+					automaticNameDelimiter: "---",
+					reuseExistingChunk: true,
+					minSize: 1,
+					maxSize: 1
+				}
+			}
 		}
 	}
 };
diff --git a/test/configCases/split-chunks/runtime-chunk/a.js b/test/configCases/split-chunks/runtime-chunk/a.js
@@ -2,9 +2,9 @@ it("should be able to load the split chunk on demand", () => {
 	const promise = import(/* webpackChunkName: "shared" */ "./shared");
 
 	const script = document.head._children[0];
-	expect(script.src).toBe("https://test.cases/path/dep~b~shared.js");
+	expect(script.src).toBe("https://test.cases/path/dep-shared_js.js");
 
-	__non_webpack_require__("./dep~b~shared.js");
+	__non_webpack_require__("./dep-shared_js.js");
 
 	return promise;
 });
diff --git a/test/configCases/split-chunks/runtime-chunk/webpack.config.js b/test/configCases/split-chunks/runtime-chunk/webpack.config.js
@@ -10,6 +10,7 @@ module.exports = {
 		filename: "[name].js"
 	},
 	optimization: {
+		chunkIds: "named",
 		runtimeChunk: "single",
 		splitChunks: {
 			cacheGroups: {

Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ const {`
`21`	`21`
`22`	`22`	`class NamedChunkIdsPlugin {`
`23`	`23`	`constructor(options) {`
`24`		`- this.delimiter = (options && options.delimiter) \|\| "~";`
	`24`	`+ this.delimiter = (options && options.delimiter) \|\| "-";`
`25`	`25`	`this.context = options && options.context;`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ module.exports = {`
`39`	`39`	}, files: ${files.join(", ")})`
`40`	`40`	`);`
`41`	`41`	`}`
`42`		`- verifyFilenameLength(filename, bundleDetect.expectedNameLength);`
	`42`	`+ verifyFilenameLength(filename.replace(/^\d+\./, "X."), bundleDetect.expectedNameLength);`
`43`	`43`	`}`
`44`	`44`
`45`	`45`	`return "./" + filename;`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
`1`	`1`	`const c = require("./commons");`
	`2`	`+require("./c");`
`2`	`3`
`3`	`4`	`module.exports = "b" + c;`