Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Avoid redundant bundle creation #741

Merged
merged 6 commits into from
May 16, 2022
Merged

[FIX] Avoid redundant bundle creation #741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2c864c1
[FIX] Avoid redundant bundle creation
codeworrior Dec 25, 2019
c21b32d
Rename options
matz3 May 4, 2022
d81cc80
Refactoring
matz3 May 4, 2022
1ace4ea
Change log messages
matz3 May 13, 2022
6ff6d5d
Apply suggestions from code review
matz3 May 13, 2022
6a80af8
Log message should start with uppercase letter
matz3 May 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions lib/tasks/bundlers/generateComponentPreload.js
View file
Open in desktop
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
Open in desktop
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);
codeworrior marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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