[FEATURE] Minifier: Support input source maps (#780)

If a resource references a source map (for example from an earlier transpilation):
1. Use that source map for the debug variant of the resource
2. Pass it to terser so it can update it based on the transformation, preserving the mapping to the initial source

Before this change, the minifier processor always created a new source map based on the minification of the resource. This change is active by default. However, if a resource is changed before the `minify` task is executed (for example by tasks like `replaceCopyright`), we assume that the referenced source map is potentially corrupt and therefore ignore it.

JIRA: CPOUI5FOUNDATION-467

---------

Co-authored-by: Matthias Osswald <mat.osswald@sap.com>
Co-authored-by: Günter Klatt <57760635+KlattG@users.noreply.github.com>

lib/lbt/bundle/Builder.js

@@ -18,7 +18,7 @@ import BundleWriter from "./BundleWriter.js";
 import {getLogger} from "@ui5/logger";
 const log = getLogger("lbt:bundle:Builder");
 
-const sourceMappingUrlPattern = /\/\/# sourceMappingURL=(.+)\s*$/;
+const sourceMappingUrlPattern = /\/\/# sourceMappingURL=(\S+)\s*$/;
 const httpPattern = /^https?:\/\//i;
 const xmlHtmlPrePattern = /<(?:\w+:)?pre\b/;
 
@@ -332,11 +332,18 @@ class BundleBuilder {
 			throw new Error("No source map provided");
 		}
 
+		// Reminder on the structure of line-segments in the map:
+		// [generatedCodeColumn, sourceIndex, sourceCodeLine, sourceCodeColumn, nameIndex]
 		if (map.mappings.startsWith(";")) {
 			// If first line is not already mapped (typical for comments or parentheses), add a mapping to
 			// make sure that dev-tools (especially Chrome's) don't choose the end of the preceding module
 			// when the user tries to set a breakpoint from the bundle file
 			map.mappings = "AAAA" + map.mappings;
+		} else if (this.outW.columnOffset === 0 && !map.mappings.startsWith("A")) {
+			// If first column of the first line is not already mapped, add a mapping for the same reason as above.
+			// This is typical for transpiled code, where there is a bunch of generated code at the beginning that
+			// can't be mapped to the original source
+			map.mappings = "AAAA," + map.mappings;
 		}
 
 		map.sourceRoot = path.posix.relative(
@@ -592,9 +599,7 @@ class BundleBuilder {
 				`Attempting to find a source map resource based on the module's path: ${sourceMapFileCandidate}`);
 			try {
 				const sourceMapResource = await this.pool.findResource(sourceMapFileCandidate);
-				if (sourceMapResource) {
-					moduleSourceMap = (await sourceMapResource.buffer()).toString();
-				}
+				moduleSourceMap = (await sourceMapResource.buffer()).toString();
 			} catch (e) {
 				// No input source map
 				log.silly(`Could not find a source map for module ${moduleName}: ${e.message}`);

lib/processors/minifier.js

@@ -1,5 +1,6 @@
 import {fileURLToPath} from "node:url";
 import posixPath from "node:path/posix";
+import {promisify} from "node:util";
 import os from "node:os";
 import workerpool from "workerpool";
 import Resource from "@ui5/fs/Resource";
@@ -13,6 +14,9 @@ const MAX_WORKERS = 4;
 const osCpus = os.cpus().length || 1;
 const maxWorkers = Math.max(Math.min(osCpus - 1, MAX_WORKERS), MIN_WORKERS);
 
+const sourceMappingUrlPattern = /\/\/# sourceMappingURL=(\S+)\s*$/;
+const httpPattern = /^https?:\/\//i;
+
 // Shared workerpool across all executions until the taskUtil cleanup is triggered
 let pool;
 
@@ -38,6 +42,63 @@ async function minifyInWorker(options, taskUtil) {
 	return getPool(taskUtil).exec("execMinification", [options]);
 }
 
+async function extractAndRemoveSourceMappingUrl(resource) {
+	const resourceContent = await resource.getString();
+	const resourcePath = resource.getPath();
+	const sourceMappingUrlMatch = resourceContent.match(sourceMappingUrlPattern);
+	if (sourceMappingUrlMatch) {
+		const sourceMappingUrl = sourceMappingUrlMatch[1];
+		if (log.isLevelEnabled("silly")) {
+			log.silly(`Found source map reference in content of resource ${resourcePath}: ${sourceMappingUrl}`);
+		}
+
+		// Strip sourceMappingURL from the resource to be minified
+		// It is not required anymore and will be replaced for in the minified resource
+		// and its debug variant anyways
+		resource.setString(resourceContent.replace(sourceMappingUrlPattern, ""));
+		return sourceMappingUrl;
+	}
+	return null;
+}
+
+async function getSourceMapFromUrl({sourceMappingUrl, resourcePath, readFile}) {
+	// =======================================================================
+	// This code is almost identical to code located in lbt/bundle/Builder.js
+	// Please try to update both places when making changes
+	// =======================================================================
+	if (sourceMappingUrl.startsWith("data:")) {
+		// Data-URI indicates an inline source map
+		const expectedTypeAndEncoding = "data:application/json;charset=utf-8;base64,";
+		if (sourceMappingUrl.startsWith(expectedTypeAndEncoding)) {
+			const base64Content = sourceMappingUrl.slice(expectedTypeAndEncoding.length);
+			// Create a resource with a path suggesting it's the source map for the resource
+			// (which it is but inlined)
+			return Buffer.from(base64Content, "base64").toString();
+		} else {
+			log.warn(
+				`Source map reference in resource ${resourcePath} is a data URI but has an unexpected` +
+				`encoding: ${sourceMappingUrl}. Expected it to start with ` +
+				`"data:application/json;charset=utf-8;base64,"`);
+		}
+	} else if (httpPattern.test(sourceMappingUrl)) {
+		log.warn(`Source map reference in resource ${resourcePath} is an absolute URL. ` +
+			`Currently, only relative URLs are supported.`);
+	} else if (posixPath.isAbsolute(sourceMappingUrl)) {
+		log.warn(`Source map reference in resource ${resourcePath} is an absolute path. ` +
+			`Currently, only relative paths are supported.`);
+	} else {
+		const sourceMapPath = posixPath.join(posixPath.dirname(resourcePath), sourceMappingUrl);
+
+		try {
+			const sourceMapContent = await readFile(sourceMapPath);
+			return sourceMapContent.toString();
+		} catch (e) {
+			// No input source map
+			log.warn(`Unable to read source map for resource ${resourcePath}: ${e.message}`);
+		}
+	}
+}
+
 /**
  * @public
  * @module @ui5/builder/processors/minifier
@@ -62,18 +123,31 @@ async function minifyInWorker(options, taskUtil) {
  *
  * @param {object} parameters Parameters
  * @param {@ui5/fs/Resource[]} parameters.resources List of resources to be processed
+ * @param {fs|module:@ui5/fs/fsInterface} parameters.fs Node fs or custom
+ *    [fs interface]{@link module:@ui5/fs/fsInterface}. Required when setting "readSourceMappingUrl" to true
  * @param {@ui5/builder/tasks/TaskUtil|object} [parameters.taskUtil] TaskUtil instance.
  *    Required when using the <code>useWorkers</code> option
  * @param {object} [parameters.options] Options
+ * @param {boolean} [parameters.options.readSourceMappingUrl=false]
+ *   Whether to make use of any existing source maps referenced in the resources to be minified. Use this option to
+ *   preserve references to the original source files, such as TypeScript files, in the generated source map.<br>
+ *   If a resource has been modified by a previous task, any existing source map will be ignored regardless of this
+ *    setting. This is to ensure that no inconsistent source maps are used. Check the verbose log for details.
  * @param {boolean} [parameters.options.addSourceMappingUrl=true]
  *   Whether to add a sourceMappingURL reference to the end of the minified resource
  * @param {boolean} [parameters.options.useWorkers=false]
  *  Whether to offload the minification task onto separate CPU threads. This often speeds up the build process
  * @returns {Promise<module:@ui5/builder/processors/minifier~MinifierResult[]>}
  *   Promise resolving with object of resource, dbgResource and sourceMap
  */
-export default async function({resources, taskUtil, options: {addSourceMappingUrl = true, useWorkers = false} = {}}) {
+export default async function({
+	resources, fs, taskUtil, options: {readSourceMappingUrl = false, addSourceMappingUrl = true, useWorkers = false
+	} = {}}) {
 	let minify;
+	if (readSourceMappingUrl && !fs) {
+		throw new Error(`Option 'readSourceMappingUrl' requires parameter 'fs' to be provided`);
+	}
+
 	if (useWorkers) {
 		if (!taskUtil) {
 			// TaskUtil is required for worker support
@@ -86,20 +160,72 @@ export default async function({resources, taskUtil, options: {addSourceMappingUr
 	}
 
 	return Promise.all(resources.map(async (resource) => {
-		const dbgPath = resource.getPath().replace(debugFileRegex, "-dbg$1");
-		const dbgResource = await resource.clone();
-		dbgResource.setPath(dbgPath);
+		const resourcePath = resource.getPath();
+		const dbgPath = resourcePath.replace(debugFileRegex, "-dbg$1");
+		const dbgFilename = posixPath.basename(dbgPath);
 
 		const filename = posixPath.basename(resource.getPath());
-		const code = await resource.getString();
 
 		const sourceMapOptions = {
 			filename
 		};
 		if (addSourceMappingUrl) {
 			sourceMapOptions.url = filename + ".map";
 		}
-		const dbgFilename = posixPath.basename(dbgPath);
+
+		// Remember contentModified flag before making changes to the resource via setString
+		const resourceContentModified = resource.getSourceMetadata()?.contentModified;
+
+		// In any case: Extract *and remove* source map reference from resource before cloning it
+		const sourceMappingUrl = await extractAndRemoveSourceMappingUrl(resource);
+
+		const code = await resource.getString();
+		// Create debug variant based off the original resource before minification
+		const dbgResource = await resource.clone();
+		dbgResource.setPath(dbgPath);
+
+		let dbgSourceMapResource;
+		if (sourceMappingUrl) {
+			if (resourceContentModified) {
+				log.verbose(
+					`Source map found in resource will be ignored because the resource has been ` +
+					`modified in a previous task: ${resourcePath}`);
+			} else if (readSourceMappingUrl) {
+				// Try to find a source map reference in the to-be-minified resource
+				// If we find one, provide it to terser as an input source map and keep using it for the
+				// debug variant of the resource
+				const sourceMapContent = await getSourceMapFromUrl({
+					sourceMappingUrl,
+					resourcePath,
+					readFile: promisify(fs.readFile)
+				});
+
+				if (sourceMapContent) {
+					// Provide source map to terser as "input source map"
+					sourceMapOptions.content = sourceMapContent;
+
+					// Also use the source map for the debug variant of the resource
+					// First update the file reference within the source map
+					const sourceMapJson = JSON.parse(sourceMapContent);
+					sourceMapJson.file = dbgFilename;
+
+					// Then create a new resource
+					dbgSourceMapResource = new Resource({
+						string: JSON.stringify(sourceMapJson),
+						path: dbgPath + ".map"
+					});
+					dbgResource.setString(code + `//# sourceMappingURL=${dbgFilename}.map\n`);
+				}
+			} else {
+				// If the original resource content was unmodified and the input source map was not parsed,
+				// re-add the original source map reference to the debug variant
+				if (!sourceMappingUrl.startsWith("data:") && !sourceMappingUrl.endsWith(filename + ".map")) {
+					// Do not re-add inline source maps as well as references to the source map of
+					// the minified resource
+					dbgResource.setString(code + `//# sourceMappingURL=${sourceMappingUrl}\n`);
+				}
+			}
+		}
 
 		const result = await minify({
 			filename,
@@ -112,6 +238,9 @@ export default async function({resources, taskUtil, options: {addSourceMappingUr
 			path: resource.getPath() + ".map",
 			string: result.map
 		});
-		return {resource, dbgResource, sourceMapResource};
+		return {resource, dbgResource, sourceMapResource, dbgSourceMapResource};
 	}));
 }
+
+export const __localFunctions__ = (process.env.NODE_ENV === "test") ?
+	{getSourceMapFromUrl} : undefined;

lib/tasks/minify.js

@@ -1,4 +1,5 @@
 import minifier from "../processors/minifier.js";
+import fsInterface from "@ui5/fs/fsInterface";
 
 /**
  * @public
@@ -19,32 +20,48 @@ import minifier from "../processors/minifier.js";
  * @param {string} parameters.options.pattern Pattern to locate the files to be processed
  * @param {boolean} [parameters.options.omitSourceMapResources=false] Whether source map resources shall
  * 		be tagged as "OmitFromBuildResult" and no sourceMappingURL shall be added to the minified resource
+ * @param {boolean} [parameters.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 {Promise<undefined>} Promise resolving with <code>undefined</code> once data has been written
  */
-export default async function({workspace, taskUtil, options: {pattern, omitSourceMapResources = false}}) {
+export default async function({
+	workspace, taskUtil, options: {pattern, omitSourceMapResources = false, useInputSourceMaps = true
+	}}) {
 	const resources = await workspace.byGlob(pattern);
 	const processedResources = await minifier({
 		resources,
+		fs: fsInterface(workspace),
 		taskUtil,
 		options: {
 			addSourceMappingUrl: !omitSourceMapResources,
+			readSourceMappingUrl: !!useInputSourceMaps,
 			useWorkers: !!taskUtil,
 		}
 	});
 
-	return Promise.all(processedResources.map(async ({resource, dbgResource, sourceMapResource}) => {
+	return Promise.all(processedResources.map(async ({
+		resource, dbgResource, sourceMapResource, dbgSourceMapResource
+	}) => {
 		if (taskUtil) {
 			taskUtil.setTag(resource, taskUtil.STANDARD_TAGS.HasDebugVariant);
 			taskUtil.setTag(dbgResource, taskUtil.STANDARD_TAGS.IsDebugVariant);
 			taskUtil.setTag(sourceMapResource, taskUtil.STANDARD_TAGS.HasDebugVariant);
 			if (omitSourceMapResources) {
 				taskUtil.setTag(sourceMapResource, taskUtil.STANDARD_TAGS.OmitFromBuildResult);
 			}
+			if (dbgSourceMapResource) {
+				taskUtil.setTag(dbgSourceMapResource, taskUtil.STANDARD_TAGS.IsDebugVariant);
+				if (omitSourceMapResources) {
+					taskUtil.setTag(dbgSourceMapResource, taskUtil.STANDARD_TAGS.OmitFromBuildResult);
+				}
+			}
 		}
 		return Promise.all([
 			workspace.write(resource),
 			workspace.write(dbgResource),
-			workspace.write(sourceMapResource)
+			workspace.write(sourceMapResource),
+			dbgSourceMapResource && workspace.write(dbgSourceMapResource)
 		]);
 	}));
 }

package.json

@@ -137,6 +137,7 @@
 	},
 	"devDependencies": {
 		"@istanbuljs/esm-loader-hook": "^0.2.0",
+		"@jridgewell/trace-mapping": "^0.3.19",
 		"@ui5/project": "^3.7.1",
 		"ava": "^5.3.1",
 		"chai": "^4.3.10",
@@ -150,6 +151,7 @@
 		"eslint-plugin-ava": "^14.0.0",
 		"eslint-plugin-jsdoc": "^46.8.2",
 		"esmock": "^2.5.1",
+		"line-column": "^1.0.2",
 		"nyc": "^15.1.0",
 		"open-cli": "^7.2.0",
 		"recursive-readdir": "^2.2.3",