fix(builder): Improvement of flexBundle flagging in generateFlexBundle

packages/builder/lib/processors/bundlers/flexChangesBundler.js

@@ -7,6 +7,20 @@ import {createResource} from "@ui5/fs/resourceFactory";
  * @module @ui5/builder/processors/bundlers/flexChangesBundler
  */
 
+/**
+ * Result of the flex changes bundling process.
+ *
+ * @public
+ * @typedef {object} FlexChangesBundlerResult
+ * @property {boolean|undefined} flexBundle Flag indicating whether a flexibility bundle was created.
+ *   - `true`: a flexibility bundle was created and at least one annotation change is included
+ *   - `undefined`: a flexibility bundle was created, but does not contain any annotation change,
+ *     so the client should not enforce an early dedicated loading of the bundle
+ *   - `false`: no flexibility bundle was created (no changes to bundle, e.g. only filtered
+ *     app-descriptor changes were provided)
+ * @property {@ui5/fs/Resource[]} bundleResources List of created flex changes bundle resources
+ */
+
 /**
  * Bundles all supplied changes.
  *
@@ -18,14 +32,18 @@ import {createResource} from "@ui5/fs/resourceFactory";
  * @param {@ui5/fs/Resource[]} parameters.resources List of resources to be processed
  * @param {object} parameters.options Options
  * @param {string} parameters.options.pathPrefix Prefix for bundle path
- * @param {string} parameters.options.hasFlexBundleVersion true if minUI5Version >= 1.73
+ * @param {boolean} parameters.options.hasFlexBundleVersion true if minUI5Version >= 1.73
  *															and create flexibility-bundle.json
  * @param {object} [parameters.existingFlexBundle={}] Object with existing flexibility-bundle.json
  * 															to merge with new changes
- * @returns {Promise<@ui5/fs/Resource[]>} Promise resolving with flex changes bundle resources
+ * @returns {Promise<FlexChangesBundlerResult>} Promise resolving with an object containing
+ *   the flexBundle flag and the created flex changes bundle resources
  */
 export default function({resources, options: {pathPrefix, hasFlexBundleVersion}, existingFlexBundle = {}}) {
 	let bundleName = "changes-bundle.json";
+	// Default to `false`: no bundle content will be written.
+	// Updated below to `true` (annotation changes present) or `undefined` otherwise
+	let flexBundle = false;
 
 	function sortByTimeStamp(a, b) {
 		return a.creation > b.creation ? 1 : -1;
@@ -34,8 +52,8 @@ export default function({resources, options: {pathPrefix, hasFlexBundleVersion},
 	/**
 	 * bundle changes resource to json string
 	 *
-	 * @param {Array} changesContent Array of resources files
-	 * @returns {string} Json sting of changes and control variants
+	 * @param {Array} changesContent Array of resources files (changes, variants, etc)
+	 * @returns {string} Json string of changes and control variants
 	 */
 	function sortAndStringifyInFlexFormat(changesContent) {
 		changesContent = changesContent.sort(sortByTimeStamp);
@@ -81,6 +99,15 @@ export default function({resources, options: {pathPrefix, hasFlexBundleVersion},
 			}
 		});
 
+		const hasBundleContent =
+			changes.length > 0 ||
+			variantDependentControlChanges.length > 0 ||
+			compVariants.length > 0 ||
+			variants.length > 0 ||
+			variantChanges.length > 0 ||
+			variantManagementChanges.length > 0 ||
+			annotationChanges.length > 0;
+
 		if (!hasFlexBundleVersion && (
 			compVariants.length != 0 ||
 			variants.length != 0 ||
@@ -93,6 +120,13 @@ export default function({resources, options: {pathPrefix, hasFlexBundleVersion},
 				"There are some files in the changes folder supported only with a UI5 version 1.73 and above. " +
 				"Please update the minUI5Version in the manifest.json to 1.73 or higher");
 		}
+
+		if (annotationChanges.length > 0) {
+			flexBundle = true;
+		} else if (hasBundleContent) {
+			flexBundle = undefined;
+		}
+
 		// create changes-bundle.json
 		if (!hasFlexBundleVersion) {
 			return JSON.stringify(changes);
@@ -149,6 +183,9 @@ export default function({resources, options: {pathPrefix, hasFlexBundleVersion},
 				string: changesContent
 			}));
 		}
-		return result;
+		return {
+			bundleResources: result,
+			flexBundle
+		};
 	});
 }

packages/builder/lib/tasks/bundlers/generateFlexChangesBundle.js

@@ -10,7 +10,7 @@ import semver from "semver";
 
 /* eslint "jsdoc/check-param-names": ["error", {"disableExtraPropertyReporting":true}] */
 /**
- * Task to create changesBundle.json file containing all changes stored in the /changes folder for easier consumption
+ * Task to create changes-bundle.json file containing all changes stored in the /changes folder for easier consumption
  * at runtime.
  * If a change bundle is created, "sap.ui.fl" is added as a dependency to the manifest.json if not already present -
  * if the dependency is already listed but lazy-loaded, lazy loading is disabled.
@@ -38,12 +38,24 @@ export default async function({workspace, taskUtil, options = {}}) {
 		pathPrefix = `/resources/${namespace}`;
 	}
 
-	function updateJson(data, bBundleCreated) {
+	/**
+	 * Update the manifest.json JSON in-place with the flexBundle flag and, if a bundle was
+	 * written, ensure the sap.ui.fl dependency is present (and lazy loading disabled).
+	 *
+	 * @param {object} data Parsed manifest.json content
+	 * @param {boolean|undefined} flexBundleFlag Result flag from the flex changes bundler:
+	 *   - `true`: bundle written including annotation changes
+	 *   - `undefined`: bundle written but without annotation changes
+	 *   - `false`: no bundle written
+	 */
+	function updateJson(data, flexBundleFlag) {
 		data["sap.ui5"] = data["sap.ui5"] || {};
 		// explicit set the flag to inform the runtime if a bundle is present before the preload can be interpreted
-		data["sap.ui5"].flexBundle = bBundleCreated;
+		data["sap.ui5"].flexBundle = flexBundleFlag;
 
-		if (bBundleCreated) {
+		// A bundle has been written whenever the flag is not explicitly `false` - this includes the `undefined`.
+		// In all these cases the sap.ui.fl library dependency is required.
+		if (flexBundleFlag !== false) {
 			// ensure the existence of the libs section in the dependencies
 			data["sap.ui5"].dependencies = data["sap.ui5"].dependencies || {};
 			const mLibs = data["sap.ui5"].dependencies.libs = data["sap.ui5"].dependencies.libs || {};
@@ -61,15 +73,15 @@ export default async function({workspace, taskUtil, options = {}}) {
 		}
 	}
 
-	async function updateManifestWithFlDependencyAndFlexBundleFlag(bBundleCreated) {
+	async function updateManifestWithFlDependencyAndFlexBundleFlag(flexBundleFlag) {
 		const manifestResource = await workspace.byPath(`${pathPrefix}/manifest.json`);
 		if (!manifestResource) {
 			log.verbose("No manifest.json found, skipping update of sap.ui.fl dependency and flexBundle flag");
 			return;
 		}
 		const manifestContent = JSON.parse(await manifestResource.getString());
 
-		updateJson(manifestContent, bBundleCreated);
+		updateJson(manifestContent, flexBundleFlag);
 		manifestResource.setString(JSON.stringify(manifestContent, null, "\t"));
 
 		await workspace.write(manifestResource);
@@ -99,7 +111,7 @@ export default async function({workspace, taskUtil, options = {}}) {
 			"ctrl_variant_change,ctrl_variant_management_change}"
 	);
 
-	let bBundleCreated = false;
+	let flexChangesBundleResult;
 
 	if (allResources.length > 0) {
 		const versionArray = await readManifestMinUI5Version();
@@ -114,20 +126,21 @@ export default async function({workspace, taskUtil, options = {}}) {
 				flexBundle = JSON.parse(await flexBundleResource.getString());
 			}
 		}
-		const processedResources = await flexChangesBundler({
+		flexChangesBundleResult = await flexChangesBundler({
 			resources: allResources,
 			options: {
 				pathPrefix,
 				hasFlexBundleVersion
 			},
 			existingFlexBundle: flexBundle
 		});
-		await Promise.all(processedResources.map((resource) => {
+
+		const bundleResources = flexChangesBundleResult.bundleResources;
+
+		await Promise.all(bundleResources.map((resource) => {
 			log.verbose("Writing flexibility changes bundle");
 			return workspace.write(resource);
 		}));
-		// Add the sap.ui.fl dependency if a bundle has been created
-		bBundleCreated = processedResources.length > 0;
 
 		// Do not write bundled source files to build result
 		if (taskUtil) {
@@ -137,6 +150,9 @@ export default async function({workspace, taskUtil, options = {}}) {
 		}
 	}
 
-	// Always append the flexBundle flag to the manifest.json, even if no bundle was created
-	await updateManifestWithFlDependencyAndFlexBundleFlag(bBundleCreated);
+	// Always append the flexBundle flag to the manifest.json, even if no bundle was created.
+	// When no flex changes have been processed at all, no bundle is created and the flag
+	// is set to `false`.
+	const flexBundleFlag = flexChangesBundleResult ? flexChangesBundleResult.flexBundle : false;
+	await updateManifestWithFlDependencyAndFlexBundleFlag(flexBundleFlag);
 }

packages/builder/test/expected/build/application.i/dest/manifest.json

@@ -21,7 +21,6 @@
 				"sap.m": {},
 				"sap.ui.fl": {}
 			}
-		},
-		"flexBundle": true
+		}
 	}
 }

packages/builder/test/lib/processors/bundlers/flexChangesBundler.js

@@ -4,7 +4,7 @@ import flexChangesBundler from "../../../../lib/processors/bundlers/flexChangesB
 test("flexChangesBundler with empty resources", async (t) => {
 	const resources = [];
 	const options = {};
-	const aResult = await flexChangesBundler({resources, options});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options});
 	t.deepEqual(aResult, [], "The result should be an empty array");
 });
 
@@ -98,7 +98,7 @@ test("flexChangesBundler with 2 changes", async (t) => {
 	const options = {
 		pathPrefix: "/mypath"
 	};
-	const aResult = await flexChangesBundler({resources, options});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options});
 	t.is(aResult.length, 1, "There should be only one element");
 	const oResult = aResult[0];
 
@@ -131,7 +131,7 @@ test("includes annotation_changes in flexibility-bundle when hasFlexBundleVersio
 	}];
 
 	const options = {pathPrefix: "/mypath", hasFlexBundleVersion: true};
-	const aResult = await flexChangesBundler({resources, options});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options});
 	t.is(aResult.length, 1);
 	const parsed = JSON.parse(await aResult[0].getString());
 	t.true(Array.isArray(parsed.annotationChanges));
@@ -450,7 +450,7 @@ test("flexChangesBundler has ctrl_variant and hasFlexBundleVersion = true", asyn
 		pathPrefix: "/mypath",
 		hasFlexBundleVersion: true
 	};
-	const aResult = await flexChangesBundler({resources, options});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options});
 	t.is(aResult.length, 1, "There should be only one element");
 	const oResult = aResult[0];
 
@@ -993,7 +993,7 @@ test("flexChangesBundler with existing flexibility-bundle.json", async (t) => {
 		pathPrefix: "/mypath",
 		hasFlexBundleVersion: true
 	};
-	const aResult = await flexChangesBundler({resources, options, existingFlexBundle});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options, existingFlexBundle});
 
 	t.is(aResult.length, 1, "There should be only one element");
 	const oResult = aResult[0];
@@ -1296,7 +1296,7 @@ test("flexChangesBundler with existing flexibility-bundle.json and missing/wrong
 		pathPrefix: "/mypath",
 		hasFlexBundleVersion: true
 	};
-	const aResult = await flexChangesBundler({resources, options, existingFlexBundle});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options, existingFlexBundle});
 
 	t.is(aResult.length, 1, "There should be only one element");
 	const oResult = aResult[0];
@@ -1407,7 +1407,7 @@ test("flexChangesBundler with existing flexibility-bundle.json and version lower
 		pathPrefix: "/mypath",
 		hasFlexBundleVersion: false
 	};
-	const aResult = await flexChangesBundler({resources, options, existingFlexBundle});
+	const {bundleResources: aResult} = await flexChangesBundler({resources, options, existingFlexBundle});
 
 	t.is(aResult.length, 1, "There should be only one element");
 	const oResult = aResult[0];