Skip to content

Commit

Permalink
[FIX] Avoid redundant bundle creation (#741)
Browse files Browse the repository at this point in the history 
When a custom bundle has the same name as a built-in bundle (e.g.
foo/bar/library-preload.js), then the creation of the built-in bundle
can be skipped.

Co-authored-by: Frank Weigel <frank.weigel@sap.com>
Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>
  • Loading branch information
@matz3 @codeworrior @RandomByte
3 people authored May 16, 2022
1 parent 042d82f commit 13c8405
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 79 deletions.
13 changes: 10 additions & 3 deletions lib/tasks/bundlers/generateComponentPreload.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@ const {negateFilters} = require("../../lbt/resources/ResourceFilterList");
* inclusion overrides an earlier exclusion, and vice versa.
* @param {string[]} [parameters.options.paths] Array of paths (or glob patterns) for component files
* @param {string[]} [parameters.options.namespaces] Array of component namespaces
* @param {string[]} [parameters.options.skipBundles] Names of bundles that should not be created
* @returns {Promise<undefined>} Promise resolving with <code>undefined</code> once data has been written
*/
module.exports = function({
workspace, taskUtil, options: {projectName, paths, namespaces, excludes = []}
workspace, taskUtil, options: {projectName, paths, namespaces, skipBundles = [], excludes = []}
}) {
let nonDbgWorkspace = workspace;
if (taskUtil) {
Expand Down Expand Up @@ -65,6 +66,12 @@ module.exports = function({
const unusedFilterExcludes = new Set(allFilterExcludes);

const bundleDefinitions = allNamespaces.map((namespace) => {
const bundleName = `${namespace}/Component-preload.js`;
if (skipBundles.includes(bundleName)) {
log.verbose(`Skipping generation of bundle ${bundleName}`);
return null;
}

const filters = [
`${namespace}/`,
`${namespace}/**/manifest.json`,
Expand Down Expand Up @@ -94,7 +101,7 @@ module.exports = function({
});

return {
name: `${namespace}/Component-preload.js`,
name: bundleName,
defaultFileTypes: [
".js",
".control.xml",
Expand Down Expand Up @@ -127,7 +134,7 @@ module.exports = function({
});
}

return Promise.all(bundleDefinitions.map((bundleDefinition) => {
return Promise.all(bundleDefinitions.filter(Boolean).map((bundleDefinition) => {
log.verbose(`Generating ${bundleDefinition.name}...`);
return moduleBundler({
resources,
Expand Down
143 changes: 73 additions & 70 deletions lib/tasks/bundlers/generateLibraryPreload.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,48 +142,6 @@ function getSupportFilesBundleDefinition(namespace) {
};
}

function createLibraryBundles(libraryNamespace, resources, excludes) {
return Promise.all([
moduleBundler({
options: {
bundleDefinition: getBundleDefinition(libraryNamespace, excludes),
bundleOptions: {
optimize: true,
usePredefineCalls: true,
ignoreMissingModules: true
}
},
resources
}),
moduleBundler({
options: {
bundleDefinition: getDesigntimeBundleDefinition(libraryNamespace),
bundleOptions: {
optimize: true,
usePredefineCalls: true,
ignoreMissingModules: true,
skipIfEmpty: true
}
},
resources
}),
moduleBundler({
options: {
bundleDefinition: getSupportFilesBundleDefinition(libraryNamespace),
bundleOptions: {
optimize: false,
usePredefineCalls: true,
ignoreMissingModules: true,
skipIfEmpty: true
}
// Note: Although the bundle uses optimize=false, there is
// no moduleNameMapping needed, as support files are excluded from minification.
},
resources
})
]);
}

function getModuleBundlerOptions(config) {
const moduleBundlerOptions = {};

Expand Down Expand Up @@ -274,9 +232,10 @@ function getSapUiCoreBunDef(name, filters, preload) {
* inclusion overrides an earlier exclusion, and vice versa.
* @param {object} parameters.options Options
* @param {string} parameters.options.projectName Project name
* @param {string[]} [parameters.options.skipBundles] Names of bundles that should not be created
* @returns {Promise<undefined>} Promise resolving with <code>undefined</code> once data has been written
*/
module.exports = function({workspace, taskUtil, options: {projectName, excludes = []}}) {
module.exports = function({workspace, taskUtil, options: {projectName, skipBundles = [], excludes = []}}) {
let nonDbgWorkspace = workspace;
if (taskUtil) {
nonDbgWorkspace = workspace.filter(function(resource) {
Expand All @@ -285,6 +244,14 @@ module.exports = function({workspace, taskUtil, options: {projectName, excludes
});
}

const execModuleBundlerIfNeeded = ({options, resources}) => {
if (skipBundles.includes(options.bundleDefinition.name)) {
log.verbose(`Skipping generation of bundle ${options.bundleDefinition.name}`);
return null;
}
return moduleBundler({options, resources});
};

return nonDbgWorkspace.byGlob("/**/*.{js,json,xml,html,properties,library,js.map}").then(async (resources) => {
// Find all libraries and create a library-preload.js bundle

Expand Down Expand Up @@ -326,32 +293,32 @@ module.exports = function({workspace, taskUtil, options: {projectName, excludes
filters = ["jquery.sap.global.js"];
}
p = Promise.all([
moduleBundler({
execModuleBundlerIfNeeded({
options: getModuleBundlerOptions({name: "sap-ui-core.js", filters, preload: true}),
resources
}),
moduleBundler({
execModuleBundlerIfNeeded({
options: getModuleBundlerOptions({
name: "sap-ui-core-dbg.js", filters, preload: false,
moduleNameMapping: unoptimizedModuleNameMapping
}),
resources: unoptimizedResources
}),
moduleBundler({
execModuleBundlerIfNeeded({
options: getModuleBundlerOptions({
name: "sap-ui-core-nojQuery.js", filters, preload: true, provided: true
}),
resources
}),
moduleBundler({
execModuleBundlerIfNeeded({
options: getModuleBundlerOptions({
name: "sap-ui-core-nojQuery-dbg.js", filters, preload: false, provided: true,
moduleNameMapping: unoptimizedModuleNameMapping
}),
resources: unoptimizedResources
}),
]).then((results) => {
const bundles = Array.prototype.concat.apply([], results);
const bundles = Array.prototype.concat.apply([], results).filter(Boolean);
return Promise.all(bundles.map(({bundle, sourceMap}) => {
if (taskUtil) {
taskUtil.setTag(bundle, taskUtil.STANDARD_TAGS.IsBundle);
Expand Down Expand Up @@ -390,7 +357,7 @@ module.exports = function({workspace, taskUtil, options: {projectName, excludes
return;
}

return Promise.all(libraryIndicatorResources.map((libraryIndicatorResource) => {
return Promise.all(libraryIndicatorResources.map(async (libraryIndicatorResource) => {
// Determine library namespace from library indicator file path
// ending with either ".library" or "library.js" (see fallback logic above)
// e.g. /resources/sap/foo/.library => sap/foo
Expand All @@ -400,28 +367,64 @@ module.exports = function({workspace, taskUtil, options: {projectName, excludes
const libraryNamespaceMatch = libraryIndicatorPath.match(libraryNamespacePattern);
if (libraryNamespaceMatch && libraryNamespaceMatch[1]) {
const libraryNamespace = libraryNamespaceMatch[1];
return createLibraryBundles(libraryNamespace, resources, excludes)
.then((results) => {
const bundles = Array.prototype.concat.apply([], results);
return Promise.all(bundles.map(({bundle, sourceMap} = {}) => {
if (bundle) {
if (taskUtil) {
taskUtil.setTag(bundle, taskUtil.STANDARD_TAGS.IsBundle);
if (sourceMap) {
// Clear tag that might have been set by the minify task, in cases where
// the bundle name is identical to a source file
taskUtil.clearTag(sourceMap,
taskUtil.STANDARD_TAGS.OmitFromBuildResult);
}
}
const writes = [workspace.write(bundle)];
if (sourceMap) {
writes.push(workspace.write(sourceMap));
}
return Promise.all(writes);
const results = await Promise.all([
execModuleBundlerIfNeeded({
options: {
bundleDefinition: getBundleDefinition(libraryNamespace, excludes),
bundleOptions: {
optimize: true,
usePredefineCalls: true,
ignoreMissingModules: true
}
},
resources
}),
execModuleBundlerIfNeeded({
options: {
bundleDefinition: getDesigntimeBundleDefinition(libraryNamespace),
bundleOptions: {
optimize: true,
usePredefineCalls: true,
ignoreMissingModules: true,
skipIfEmpty: true
}
}));
});
},
resources
}),
execModuleBundlerIfNeeded({
options: {
bundleDefinition: getSupportFilesBundleDefinition(libraryNamespace),
bundleOptions: {
optimize: false,
usePredefineCalls: true,
ignoreMissingModules: true,
skipIfEmpty: true
}
// Note: Although the bundle uses optimize=false, there is
// no moduleNameMapping needed, as support files are excluded from minification.
},
resources
})
]);
const bundles = Array.prototype.concat.apply([], results).filter(Boolean);
return Promise.all(bundles.map(({bundle, sourceMap} = {}) => {
if (bundle) {
if (taskUtil) {
taskUtil.setTag(bundle, taskUtil.STANDARD_TAGS.IsBundle);
if (sourceMap) {
// Clear tag that might have been set by the minify task, in cases where
// the bundle name is identical to a source file
taskUtil.clearTag(sourceMap,
taskUtil.STANDARD_TAGS.OmitFromBuildResult);
}
}
const writes = [workspace.write(bundle)];
if (sourceMap) {
writes.push(workspace.write(sourceMap));
}
return Promise.all(writes);
}
}));
} else {
log.verbose(
`Could not determine library namespace from file "${libraryIndicatorPath}" ` +
Expand Down
11 changes: 8 additions & 3 deletions lib/types/application/ApplicationBuilder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ class ApplicationBuilder extends AbstractBuilder {
});
}

const bundles = project.builder?.bundles;
const existingBundleDefinitionNames =
bundles?.map(({bundleDefinition}) => bundleDefinition.name).filter(Boolean) || [];

const componentPreload = project.builder && project.builder.componentPreload;
if (componentPreload && (componentPreload.namespaces || componentPreload.paths)) {
this.addTask("generateComponentPreload", async () => {
Expand All @@ -98,7 +102,8 @@ class ApplicationBuilder extends AbstractBuilder {
projectName: project.metadata.name,
paths: componentPreload.paths,
namespaces: componentPreload.namespaces,
excludes: componentPreload.excludes
excludes: componentPreload.excludes,
skipBundles: existingBundleDefinitionNames
}
});
});
Expand All @@ -111,7 +116,8 @@ class ApplicationBuilder extends AbstractBuilder {
options: {
projectName: project.metadata.name,
namespaces: [project.metadata.namespace],
excludes: componentPreload && componentPreload.excludes
excludes: componentPreload && componentPreload.excludes,
skipBundles: existingBundleDefinitionNames
}
});
});
Expand Down Expand Up @@ -139,7 +145,6 @@ class ApplicationBuilder extends AbstractBuilder {
});
});

const bundles = project.builder && project.builder.bundles;
if (bundles) {
this.addTask("generateBundle", async () => {
return Promise.all(bundles.map((bundle) => {
Expand Down
11 changes: 8 additions & 3 deletions lib/types/library/LibraryBuilder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ class LibraryBuilder extends AbstractBuilder {
});
}

const bundles = project.builder?.bundles;
const existingBundleDefinitionNames =
bundles?.map(({bundleDefinition}) => bundleDefinition.name).filter(Boolean) || [];

const componentPreload = project.builder && project.builder.componentPreload;
if (componentPreload) {
this.addTask("generateComponentPreload", async () => {
Expand All @@ -142,7 +146,8 @@ class LibraryBuilder extends AbstractBuilder {
projectName: project.metadata.name,
paths: componentPreload.paths,
namespaces: componentPreload.namespaces,
excludes: componentPreload.excludes
excludes: componentPreload.excludes,
skipBundles: existingBundleDefinitionNames
}
});
});
Expand All @@ -157,12 +162,12 @@ class LibraryBuilder extends AbstractBuilder {
excludes:
project.builder &&
project.builder.libraryPreload &&
project.builder.libraryPreload.excludes
project.builder.libraryPreload.excludes,
skipBundles: existingBundleDefinitionNames
}
});
});

const bundles = project.builder && project.builder.bundles;
if (bundles) {
this.addTask("generateBundle", async () => {
return bundles.reduce(function(sequence, bundle) {
Expand Down

0 comments on commit 13c8405

Please sign in to comment.