refactor(builder): Tag debug variants pairs skipped by minify

packages/builder/lib/tasks/minify.js

@@ -1,4 +1,5 @@
 import fsInterface from "@ui5/fs/fsInterface";
+import micromatch from "micromatch";
 import {getDebugName, getNonDebugName} from "../lbt/utils/ModuleName.js";
 import {getLogger} from "@ui5/logger";
 const log = getLogger("builder:tasks:minify");
@@ -16,21 +17,47 @@ const log = getLogger("builder:tasks:minify");
  * source map (the <code>//# sourceMappingURL=</code> target) is read through the step's workspace and
  * thus recorded as an input of that step, so changing only the <code>.js.map</code> re-runs its owning
  * <code>.js</code> and regenerates a correct <code>-dbg.js.map</code>.
+ * Existing debug/non-debug pairs are left unchanged and tagged, even when excluded from minification.
+ * Each pair has a separate tagging key, so its tags can be restored without re-running the callback.
  *
  * @public
  * @function default
  * @static
  *
  * @param {object} options Options
- * @param {string} options.pattern Pattern to locate the files to be processed
+ * @param {string|string[]} options.pattern Patterns defining the resource scope, including existing debug pairs
+ * @param {string[]} [options.excludes=[]] Glob patterns matched against absolute virtual resource paths.
+ *   A leading <code>!</code> re-includes matches; later entries take precedence. Patterns are used as
+ *   supplied, without a path prefix or implicit exclusions.
+ *   Exclusions do not affect discovery or tagging of existing debug pairs.
  * @param {boolean} [options.omitSourceMapResources=false] Whether source map resources shall
  * 		be tagged as "OmitFromBuildResult" and no sourceMappingURL shall be added to the minified resource
  * @param {boolean} [options.useInputSourceMaps=true] Whether to make use of any existing source
  * 		maps referenced in the resources to be minified. Use this option to preserve reference to the original
  * 		source files, such as TypeScript files, in the generated source map.
  * @returns {module:@ui5/builder/tasks/runTask~TaskDescriptor} The task descriptor with its build steps
  */
-export default function defineTask({pattern, omitSourceMapResources = false, useInputSourceMaps = true}) {
+export default function defineTask({
+	pattern, excludes = [], omitSourceMapResources = false, useInputSourceMaps = true,
+}) {
+	// Compile once per task, not once per resource or delta build. Match backwards so the last
+	// matching exclusion/re-inclusion wins.
+	const filters = excludes.map((exclude) => {
+		const include = exclude.startsWith("!");
+		const pattern = include ? exclude.slice(1) : exclude;
+		return {
+			include,
+			matches: micromatch.matcher(pattern, {dot: true}),
+		};
+	}).reverse();
+	const shouldMinify = (resourcePath) => {
+		for (const {include, matches} of filters) {
+			if (matches(resourcePath)) {
+				return include;
+			}
+		}
+		return true;
+	};
 	const minifierOptions = {
 		addSourceMappingUrl: !omitSourceMapResources,
 		readSourceMappingUrl: !!useInputSourceMaps,
@@ -68,41 +95,52 @@ export default function defineTask({pattern, omitSourceMapResources = false, use
 
 	return {steps: [{
 		name: "minify",
-		// One key per matched resource, so a delta build re-minifies only the resources whose inputs
-		// changed. The input source map is read through the step's workspace and thus tracked per step.
+		// Ordinary resource keys retain their content-based identity and source-map dependency tracking.
+		// Existing pairs use string keys and do not load/hash their content during key enumeration.
 		keys: async ({workspace}) => {
+			// The single discovery query records every input needed for pairing. Exclusions only select
+			// minification keys; they are task options and therefore already part of the build signature.
 			const resources = await workspace.byGlob(pattern);
-
-			// A -dbg.js next to its .js in the sources is unexpected: the minify task owns the -dbg.js
-			// path and generates it from the .js. Were both kept, the .js key and the -dbg.js key would
-			// write the same -dbg.js path, which the shared assertDistinctWrite guard rejects, crashing
-			// the build. So skip both members of such a pair: the .js is left as it is (the existing
-			// -dbg.js already serves as its debug variant) and the -dbg.js is passed through unchanged.
-			// A warning tells the user to exclude both from minification if the pairing is intentional.
 			const paths = new Set(resources.map((resource) => resource.getPath()));
-			return resources.filter((resource) => {
+			const minificationPaths = new Set([...paths].filter(shouldMinify));
+			const keys = [];
+			for (const resource of resources) {
 				const resourcePath = resource.getPath();
 				const dbgPath = getDebugName(resourcePath);
 				if (dbgPath && paths.has(dbgPath)) {
-					// A .js whose -dbg.js sibling already exists in the sources: skip it
+					// Neither member may be minified: their generated outputs would collide. Give the pair
+					// its own invocation so tags are replayed on a delta, and dropped when the pair disappears.
+					keys.push(JSON.stringify({resourcePath, dbgPath,
+						warn: minificationPaths.has(resourcePath) || minificationPaths.has(dbgPath)}));
+				} else if (!paths.has(getNonDebugName(resourcePath)) && minificationPaths.has(resourcePath)) {
+					keys.push(resource);
+				}
+			}
+			return keys;
+		},
+		each: async (inputResource, {workspace, taskUtil}) => {
+			if (typeof inputResource === "string") {
+				const {resourcePath, dbgPath, warn} = JSON.parse(inputResource);
+				if (warn) {
 					log.warn(
 						`Skipping minification of ${resourcePath}: a debug variant ${dbgPath} already ` +
 						`exists next to it. The minify task generates the debug variant itself, so sources ` +
 						`normally must not contain one. If this is intentional (for example the file uses ` +
 						`ECMAScript syntax the minify task does not support yet), list both ${resourcePath} ` +
 						`and ${dbgPath} in the 'builder.minification.excludes' configuration of the ` +
 						`project's ui5.yaml.`);
-					return false;
 				}
-				const nonDbgPath = getNonDebugName(resourcePath);
-				if (nonDbgPath && paths.has(nonDbgPath)) {
-					// Skip the -dbg.js itself when its non-debug .js sibling is also present
-					return false;
+				if (taskUtil) {
+					// Attribute these reads and tag operations to the pair, not the enumerator. Unrelated
+					// minification keys stay cached when a member changes; excluded pairs are never written.
+					const [resource, dbgResource] = await Promise.all([
+						workspace.byPath(resourcePath), workspace.byPath(dbgPath),
+					]);
+					taskUtil.setTag(resource, taskUtil.STANDARD_TAGS.HasDebugVariant);
+					taskUtil.setTag(dbgResource, taskUtil.STANDARD_TAGS.IsDebugVariant);
 				}
-				return true;
-			});
-		},
-		each: async (inputResource, {workspace, taskUtil}) => {
+				return;
+			}
 			// Load the minifier (and its worker-pool module graph) lazily, inside the step body, so plan-time
 			// step-name discovery can import this factory module without evaluating that graph. A build whose
 			// minify keys are all cache hits never reaches here.

packages/builder/package.json

@@ -100,6 +100,7 @@
 		"graceful-fs": "^4.2.11",
 		"jsdoc": "^4.0.5",
 		"less-openui5": "^0.12.0",
+		"micromatch": "^4.0.8",
 		"pretty-data": "^0.40.0",
 		"semver": "^7.8.5",
 		"terser": "^5.51.2",

packages/builder/test/lib/tasks/minify.excludes.js

@@ -0,0 +1,44 @@
+import test from "ava";
+import fs from "node:fs/promises";
+import path from "node:path";
+import {createAdapter, createResource, prefixGlobPattern} from "@ui5/fs/resourceFactory";
+import minify from "../../../lib/tasks/minify.js";
+
+// Compare the in-memory eligibility matcher with the old ordered glob query, including adapter
+// prefix normalization. No pairs are needed here: each returned key is an eligible input resource.
+for (const fileSystem of [false, true]) {
+	test(`minify: exclusion matching agrees with the reader (fileSystem=${fileSystem})`, async (t) => {
+		let fsBasePath;
+		if (fileSystem) {
+			const tmp = path.resolve(import.meta.dirname, "../../tmp/minify-excludes");
+			await fs.mkdir(tmp, {recursive: true});
+			fsBasePath = await fs.mkdtemp(path.join(tmp, "sources-"));
+			t.teardown(() => fs.rm(fsBasePath, {recursive: true, force: true}));
+		}
+		for (const virBasePath of ["/resources/", "/resources/ns/", "/test-resources/"]) {
+			const workspace = createAdapter({virBasePath, fsBasePath});
+			for (const name of [
+				"a.js", "b.js", "sub/a.js", ".hidden.js", "sub/.hidden.js",
+				"Rules.support.js", "sub/Rules.support.js", "name[1].js",
+			]) {
+				await workspace.write(createResource({path: virBasePath + name, string: "source"}));
+			}
+			for (const excludes of [
+				[], ["*.js"], ["**/*.js"], ["**/.*.js"],
+				["**/{a,b}.js", "!**/a.js"], ["**/+(a|b).js"], ["**/!(b).js"],
+				["**/*.js", "!**/a.js", "**/a.js"], ["!**/*.support.js"],
+				["ns/**/*.js", "!ns/a.js"], ["**/name\\[1\\].js"],
+			]) {
+				const previousPattern = ["/**/*.js", "!**/*.support.js", ...excludes.map((exclude) =>
+					exclude.startsWith("!") ? `/resources/${exclude.slice(1)}` : `!/resources/${exclude}`)];
+				const expected = await workspace.byGlob(previousPattern);
+				const [step] = minify({pattern: "/**/*.js", excludes: [
+					"**/*.support.js", ...excludes.flatMap((exclude) => prefixGlobPattern(exclude, "/resources/")),
+				]}).steps;
+				const keys = await step.keys({workspace});
+				t.deepEqual(keys.map((res) => res.getPath()).sort(), expected.map((res) => res.getPath()).sort(),
+					`Same eligibility for ${virBasePath} and ${JSON.stringify(excludes)}`);
+			}
+		}
+	});
+}