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

Jump to bottom

[FEATURE] Bundle require section with async flag for specVersion: 4.0 #1042

Merged
merged 9 commits into from
Jul 5, 2024
Merged

[FEATURE] Bundle require section with async flag for specVersion: 4.0 #1042

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c457608
Apply bundleDefinition defaults based on specVersion
d3xter666 Jul 1, 2024
1b6c921
Fix tests
d3xter666 Jul 1, 2024
e390877
add tests
d3xter666 Jul 2, 2024
4c8d22d
start all tests
d3xter666 Jul 2, 2024
702ea88
Add comments to the code
d3xter666 Jul 2, 2024
bb33e24
Add taskUtil as dependency to moduleBundler
d3xter666 Jul 2, 2024
31c2bca
Move applyDefaults to external util & adjust tests
d3xter666 Jul 2, 2024
50a1979
Remove redundant tests
d3xter666 Jul 2, 2024
7916be7
Remove unused parameter from moduleBundler
RandomByte Jul 5, 2024
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
10 changes: 2 additions & 8 deletions lib/processors/bundlers/moduleBundler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ const log = getLogger("builder:processors:bundlers:moduleBundler");
* @param {string} [parameters.options.targetUi5CoreVersion] Optional semver compliant sap.ui.core project version, e.g '2.0.0'.
This allows the bundler to make assumptions on available runtime APIs.
Omit if the ultimate UI5 version at runtime is unknown or can't be determined.
* @param {@ui5/project/build/helpers/TaskUtil|object} [parameters.taskUtil] TaskUtil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obsolete parameter? or was it already in use but undocumented?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed it in https://github.com/SAP/ui5-builder/pull/1036/files#diff-255002459f4f366a0dceae0db4927dd06d44894f887b3ba210884d9a399f48d1L138

* @returns {Promise<module:@ui5/builder/processors/bundlers/moduleBundler~ModuleBundlerResult[]>}
* Promise resolving with module bundle resources
*/
Expand All @@ -152,14 +153,7 @@ export default function({resources, options: {
ignoreMissingModules: false
}, bundleOptions);

// Apply defaults without modifying the passed object
bundleDefinition = Object.assign({
resolve: false,
resolveConditional: false,
renderer: false,
sort: true,
declareRawModules: false,
}, bundleDefinition);
// bundleDefinition's defaults get applied in the corresponding standard tasks

const pool = new LocatorResourcePool({
ignoreMissingModules: bundleOptions.ignoreMissingModules
Expand Down
5 changes: 4 additions & 1 deletion lib/tasks/bundlers/generateBundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import moduleBundler from "../../processors/bundlers/moduleBundler.js";
import {applyDefaultsToBundleDefinition} from "./utils/applyDefaultsToBundleDefinition.js";
import createModuleNameMapping from "./utils/createModuleNameMapping.js";
import ReaderCollectionPrioritized from "@ui5/fs/ReaderCollectionPrioritized";

Expand Down Expand Up @@ -96,7 +97,9 @@ export default async function({
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();
return combo.byGlob("/resources/**/*.{js,json,xml,html,properties,library,js.map}").then((resources) => {
const options = {bundleDefinition, bundleOptions};
const options = {
bundleDefinition: applyDefaultsToBundleDefinition(bundleDefinition, taskUtil), bundleOptions
};
if (!optimize && taskUtil) {
options.moduleNameMapping = createModuleNameMapping({resources, taskUtil});
}
Expand Down
3 changes: 2 additions & 1 deletion lib/tasks/bundlers/generateComponentPreload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from "node:path";
import moduleBundler from "../../processors/bundlers/moduleBundler.js";
import {applyDefaultsToBundleDefinition} from "./utils/applyDefaultsToBundleDefinition.js";
import {getLogger} from "@ui5/logger";
const log = getLogger("builder:tasks:bundlers:generateComponentPreload");
import {negateFilters} from "../../lbt/resources/ResourceFilterList.js";
Expand Down Expand Up @@ -148,7 +149,7 @@ export default async function({
return Promise.all(bundleDefinitions.filter(Boolean).map((bundleDefinition) => {
log.verbose(`Generating ${bundleDefinition.name}...`);
const options = {
bundleDefinition,
bundleDefinition: applyDefaultsToBundleDefinition(bundleDefinition, taskUtil),
bundleOptions: {
ignoreMissingModules: true,
optimize: true
Expand Down
2 changes: 2 additions & 0 deletions lib/tasks/bundlers/generateLibraryPreload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {getLogger} from "@ui5/logger";
const log = getLogger("builder:tasks:bundlers:generateLibraryPreload");
import moduleBundler from "../../processors/bundlers/moduleBundler.js";
import {applyDefaultsToBundleDefinition} from "./utils/applyDefaultsToBundleDefinition.js";
import {negateFilters} from "../../lbt/resources/ResourceFilterList.js";
import createModuleNameMapping from "./utils/createModuleNameMapping.js";

Expand Down Expand Up @@ -265,6 +266,7 @@ export default async function({workspace, taskUtil, options: {skipBundles = [],
if (coreVersion) {
options.targetUi5CoreVersion = coreVersion;
}
options.bundleDefinition = applyDefaultsToBundleDefinition(options.bundleDefinition, taskUtil);
return moduleBundler({options, resources});
};

Expand Down
33 changes: 20 additions & 13 deletions lib/tasks/bundlers/generateStandaloneAppBundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {getLogger} from "@ui5/logger";
const log = getLogger("builder:tasks:bundlers:generateStandaloneAppBundle");
import ReaderCollectionPrioritized from "@ui5/fs/ReaderCollectionPrioritized";
import moduleBundler from "../../processors/bundlers/moduleBundler.js";
import {applyDefaultsToBundleDefinition} from "./utils/applyDefaultsToBundleDefinition.js";
import createModuleNameMapping from "./utils/createModuleNameMapping.js";

function getBundleDefinition(config) {
Expand Down Expand Up @@ -139,24 +140,30 @@ export default async function({workspace, dependencies, taskUtil, options}) {
}

const bundleOptions = {
bundleDefinition: getBundleDefinition({
name: "sap-ui-custom.js",
filters,
namespace,
preloadSection: true
})
bundleDefinition: applyDefaultsToBundleDefinition(
getBundleDefinition({
name: "sap-ui-custom.js",
filters,
namespace,
preloadSection: true,
}),
taskUtil
),
};

const bundleDbgOptions = {
bundleDefinition: getBundleDefinition({
name: "sap-ui-custom-dbg.js",
filters,
namespace
}),
bundleDefinition: applyDefaultsToBundleDefinition(
getBundleDefinition({
name: "sap-ui-custom-dbg.js",
filters,
namespace,
}),
taskUtil
),
bundleOptions: {
optimize: false
optimize: false,
},
moduleNameMapping: unoptimizedModuleNameMapping
moduleNameMapping: unoptimizedModuleNameMapping,
};

if (coreVersion) {
Expand Down
31 changes: 31 additions & 0 deletions lib/tasks/bundlers/utils/applyDefaultsToBundleDefinition.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* Applies default values to bundleDefinitions
*
* @param {module:@ui5/builder/processors/bundlers/moduleBundler~ModuleBundleDefinition} bundleDefinition Module
* bundle definition
* @param {@ui5/project/build/helpers/TaskUtil|object} [taskUtil] TaskUtil
*
* @returns {module:@ui5/builder/processors/bundlers/moduleBundler~ModuleBundleDefinition}
*/
export function applyDefaultsToBundleDefinition(bundleDefinition, taskUtil) {
bundleDefinition.sections = bundleDefinition?.sections?.map((section) => {
const defaultValues = {
resolve: false,
resolveConditional: false,
renderer: false,
sort: true,
declareRawModules: false,
};

// Since specVersion: 4.0 "require" section starts loading asynchronously.
// If specVersion cannot be determined, the latest spec is taken into account.
// This is a breaking change in specVersion: 4.0
if (section.mode === "require" && (!taskUtil || taskUtil.getProject().getSpecVersion().gte("4.0"))) {
defaultValues.async = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that verifies this behavior?
The bundle tooling already defaults to "async=true", so I doubt that this line has any effect. My expectation would be that the flag has to be actively set to false in the else case, as this is the value that differs from the new default behavior.

I tested this with a self-contained build and even with specVersion 3.0 I get an async require section, which is not expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also document the "async" option in the respective JSDoc: https://github.com/SAP/ui5-builder/blob/main/lib/processors/bundlers/moduleBundler.js#L13

}

return Object.assign(defaultValues, section);
});

return bundleDefinition;
}
88 changes: 7 additions & 81 deletions test/lib/processors/bundlers/moduleBundler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,6 @@ test.serial("Builder returns single bundle", async (t) => {
const bundleOptions = {
"some": "option"
};
const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "BundleName.js",
Expand Down Expand Up @@ -103,7 +93,7 @@ test.serial("Builder returns single bundle", async (t) => {

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], {
// default bundleOptions
Expand Down Expand Up @@ -143,17 +133,6 @@ test.serial("Builder returns multiple bundles", async (t) => {
"some": "option"
};

const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundles = [
{
name: "BundleName1.js",
Expand Down Expand Up @@ -226,7 +205,7 @@ test.serial("Builder returns multiple bundles", async (t) => {

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], {
// default bundleOptions
Expand Down Expand Up @@ -268,16 +247,6 @@ test.serial("bundleOptions default (no options passed)", async (t) => {
const bundleDefinition = {
"some": "definition"
};
const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "BundleName.js",
Expand Down Expand Up @@ -325,7 +294,7 @@ test.serial("bundleOptions default (no options passed)", async (t) => {

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], {
// default bundleOptions
Expand Down Expand Up @@ -361,17 +330,6 @@ test.serial("bundleOptions default (empty options passed)", async (t) => {
};
const bundleOptions = {};

const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "BundleName.js",
content: "Bundle Content",
Expand Down Expand Up @@ -400,7 +358,7 @@ test.serial("bundleOptions default (empty options passed)", async (t) => {

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], {
// default bundleOptions
Expand Down Expand Up @@ -434,17 +392,6 @@ test.serial("bundleOptions (all options passed)", async (t) => {
ignoreMissingModules: true
};

const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "BundleName.js",
content: "Bundle Content",
Expand Down Expand Up @@ -473,7 +420,7 @@ test.serial("bundleOptions (all options passed)", async (t) => {

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], bundleOptions,
"builder.createBundle should be called with bundleOptions");
Expand Down Expand Up @@ -502,16 +449,6 @@ test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", a

"ignoreMissingModules": "foo"
};
const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "BundleName.js",
Expand Down Expand Up @@ -560,7 +497,7 @@ test.serial("Passes ignoreMissingModules bundleOption to LocatorResourcePool", a

t.is(builder.createBundle.callCount, 1, "builder.createBundle should be called once");
t.is(builder.createBundle.getCall(0).args.length, 2);
t.deepEqual(builder.createBundle.getCall(0).args[0], effectiveBundleDefinition,
t.deepEqual(builder.createBundle.getCall(0).args[0], bundleDefinition,
"builder.createBundle should be called with bundleDefinition");
t.deepEqual(builder.createBundle.getCall(0).args[1], effectiveBundleOptions,
"builder.createBundle should be called with bundleOptions");
Expand Down Expand Up @@ -602,17 +539,6 @@ test.serial("Verbose Logging", async (t) => {
"some": "option",
};

const effectiveBundleDefinition = {
// Defaults
"resolve": false,
"resolveConditional": false,
"renderer": false,
"sort": true,
"declareRawModules": false,

"some": "definition"
};

const createdBundle = {
name: "Bundle Name",
content: "Bundle Content",
Expand Down Expand Up @@ -645,6 +571,6 @@ test.serial("Verbose Logging", async (t) => {

t.deepEqual(log.verbose.getCall(0).args, ["Generating bundle:"]);
t.deepEqual(log.verbose.getCall(1).args,
["bundleDefinition: " + JSON.stringify(effectiveBundleDefinition, null, 2)]);
["bundleDefinition: " + JSON.stringify(bundleDefinition, null, 2)]);
t.deepEqual(log.verbose.getCall(2).args, ["bundleOptions: " + JSON.stringify(effectiveBundleOptions, null, 2)]);
});
Loading