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.

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

[BREAKING] Discontinue bundling of JavaScript modules as string #1036

Merged
merged 1 commit into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
67 changes: 50 additions & 17 deletions lib/lbt/bundle/Builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,13 @@ function isEmptyBundle(resolvedBundle) {
}

class BundleBuilder {
constructor(pool, targetUi5CoreVersion) {
constructor(pool, targetUi5CoreVersion, allowStringBundling) {
this.pool = pool;
this.resolver = new BundleResolver(pool);
this.splitter = new BundleSplitter(pool, this.resolver);
this.targetUi5CoreVersion = targetUi5CoreVersion;
this.targetUi5CoreVersionMajor = undefined;
this.allowStringBundling = allowStringBundling;
}

getUi5MajorVersion() {
Expand Down Expand Up @@ -172,7 +173,7 @@ class BundleBuilder {

this.closeModule(resolvedModule);

const bundleInfo = await resolvedModule.createModuleInfo(this.pool);
const bundleInfo = await resolvedModule.createModuleInfo(this.pool, this.allowStringBundling);
bundleInfo.size = this.outW.length;

return {
Expand Down Expand Up @@ -231,23 +232,23 @@ class BundleBuilder {
}
}

addSection(section) {
async addSection(section) {
this.ensureRawDeclarations();

switch (section.mode) {
case SectionType.Provided:
// do nothing
return undefined; // nothing to wait for
case SectionType.Raw:
return this.writeRaw(section);
return await this.writeRaw(section);
case SectionType.Preload:
return this.writePreloadFunction(section);
return await this.writePreloadFunction(section);
case SectionType.BundleInfo:
return this.writeBundleInfos([section]);
return await this.writeBundleInfos([section]);
case SectionType.Require:
return this.writeRequires(section);
return await this.writeRequires(section);
case SectionType.DepCache:
return this.writeDepCache(section);
return await this.writeDepCache(section);
default:
throw new Error("unknown section mode " + section.mode);
}
Expand Down Expand Up @@ -330,7 +331,6 @@ class BundleBuilder {
if ( i>0 ) {
outW.writeln(",");
}
// this.beforeWritePreloadModule(module, resource.info, resource);
outW.write(`\t"${module.toString()}":`);
outW.startSegment(module);
await this.writePreloadModule(module, resource.info, resource);
Expand Down Expand Up @@ -397,8 +397,13 @@ class BundleBuilder {
const remaining = [];
for ( const moduleName of sequence ) {
if ( /\.js$/.test(moduleName) ) {
// console.log("Processing " + moduleName);
const resource = await this.pool.findResourceWithInfo(moduleName);

if (resource.info?.requiresTopLevelScope && !this.allowStringBundling) {
this.logStringBundlingError(moduleName);
continue;
}

let moduleContent = (await resource.buffer()).toString();
moduleContent = removeHashbang(moduleContent);
let moduleSourceMap;
Expand Down Expand Up @@ -478,8 +483,8 @@ class BundleBuilder {
if (this.options.sourceMap) {
// We are actually not interested in the source map this module might contain,
// but we should make sure to remove any "sourceMappingURL" from the module content before
// writing it to the bundle. Otherwise browser dev-tools might create unnecessary (and likely incorrect)
// requests for any referenced .map files
// writing it to the bundle. Otherwise browser dev-tools might create unnecessary
// (and likely incorrect) requests for any referenced .map files
({moduleContent} =
await this.getSourceMapForModule({
moduleName,
Expand Down Expand Up @@ -542,15 +547,43 @@ class BundleBuilder {
});
}

writeBundleInfos(sections) {
logStringBundlingError(moduleName) {
log.error(
flovogt marked this conversation as resolved.
Show resolved Hide resolved
"Module " + moduleName + " requires top level scope and can only be embedded as a string " +
"(requires 'eval'), which is not supported with specVersion 4.0 and higher. " +
"For more information, see the UI5 Tooling documentation " +
"https://sap.github.io/ui5-tooling/stable/pages/Builder/#javascript-files-requiring-top-level-scope");
}

async checkForStringBundling(moduleName) {
if (!this.allowStringBundling && /\.js$/.test(moduleName)) {
const resource = await this.pool.findResourceWithInfo(moduleName);
if (resource.info?.requiresTopLevelScope) {
this.logStringBundlingError(moduleName);
return null;
}
}
return moduleName;
}

async writeBundleInfos(sections) {
this.outW.ensureNewLine();

let bundleInfoStr = "";
if ( sections.length > 0 ) {
bundleInfoStr = "sap.ui.loader.config({bundlesUI5:{\n";
sections.forEach((section, idx) => {
if ( idx > 0 ) {
let initial = true;
for (let idx = 0; idx < sections.length; idx++) {
const section = sections[idx];

// Remove modules requiring string bundling
let modules = await Promise.all(section.modules.map(this.checkForStringBundling.bind(this)));
modules = modules.filter(($) => $) || [];

if (!initial) {
bundleInfoStr += ",\n";
} else {
initial = false;
}

if (!section.name) {
Expand All @@ -561,8 +594,8 @@ class BundleBuilder {
`The info might not work as expected. ` +
`The name must match the bundle filename (incl. extension such as '.js')`);
}
bundleInfoStr += `"${section.name}":[${section.modules.map(makeStringLiteral).join(",")}]`;
});
bundleInfoStr += `"${section.name}":[${modules.map(makeStringLiteral).join(",")}]`;
}
bundleInfoStr += "\n}});\n";

this.writeWithSourceMap(bundleInfoStr);
Expand Down
19 changes: 4 additions & 15 deletions lib/lbt/bundle/ResolvedBundleDefinition.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class ResolvedBundleDefinition {
);
}

createModuleInfo(pool) {
createModuleInfo(pool, allowStringBundling) {
const bundleInfo = new ModuleInfo();
bundleInfo.name = this.name;

Expand Down Expand Up @@ -66,7 +66,9 @@ class ResolvedBundleDefinition {
modules.map( (submodule) => {
return pool.getModuleInfo(submodule).then(
(subinfo) => {
if (!bundleInfo.subModules.includes(subinfo.name)) {
if (!bundleInfo.subModules.includes(subinfo.name) &&
(!subinfo.requiresTopLevelScope ||
(subinfo.requiresTopLevelScope && allowStringBundling))) {
bundleInfo.addSubModule(subinfo);
}
}
Expand All @@ -78,19 +80,6 @@ class ResolvedBundleDefinition {

return promise.then( () => bundleInfo );
}

/*
public JSModuleDefinition getDefinition() {
return moduleDefinition;
}

public Configuration getConfiguration() {
return moduleDefinition.getConfiguration();
}


}
*/
}

class ResolvedSection {
Expand Down
6 changes: 3 additions & 3 deletions lib/processors/bundlers/moduleBundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ 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
* @param {boolean} [parameters.options.allowStringBundling=false] Optional flag to allow bundling of modules as a string.
* @returns {Promise<module:@ui5/builder/processors/bundlers/moduleBundler~ModuleBundlerResult[]>}
* Promise resolving with module bundle resources
*/
/* eslint-enable max-len */
export default function({resources, options: {
bundleDefinition, bundleOptions, moduleNameMapping, targetUi5CoreVersion
bundleDefinition, bundleOptions, moduleNameMapping, targetUi5CoreVersion, allowStringBundling = false
}}) {
// Apply defaults without modifying the passed object
bundleOptions = Object.assign({}, {
Expand All @@ -158,7 +158,7 @@ export default function({resources, options: {
const pool = new LocatorResourcePool({
ignoreMissingModules: bundleOptions.ignoreMissingModules
});
const builder = new BundleBuilder(pool, targetUi5CoreVersion);
const builder = new BundleBuilder(pool, targetUi5CoreVersion, allowStringBundling);

if (log.isLevelEnabled("verbose")) {
log.verbose(`Generating bundle:`);
Expand Down
5 changes: 4 additions & 1 deletion lib/tasks/bundlers/generateBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,12 @@ export default async function({
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();
const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
return combo.byGlob("/resources/**/*.{js,json,xml,html,properties,library,js.map}").then((resources) => {
const options = {
bundleDefinition: applyDefaultsToBundleDefinition(bundleDefinition, taskUtil), bundleOptions
bundleDefinition: applyDefaultsToBundleDefinition(bundleDefinition, taskUtil),
bundleOptions,
allowStringBundling
};
if (!optimize && taskUtil) {
options.moduleNameMapping = createModuleNameMapping({resources, taskUtil});
Expand Down
4 changes: 3 additions & 1 deletion lib/tasks/bundlers/generateComponentPreload.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,16 @@ export default async function({
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();
const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
return Promise.all(bundleDefinitions.filter(Boolean).map((bundleDefinition) => {
log.verbose(`Generating ${bundleDefinition.name}...`);
const options = {
bundleDefinition: applyDefaultsToBundleDefinition(bundleDefinition, taskUtil),
bundleOptions: {
ignoreMissingModules: true,
optimize: true
}
},
allowStringBundling
};
if (coreVersion) {
options.targetUi5CoreVersion = coreVersion;
Expand Down
3 changes: 2 additions & 1 deletion lib/tasks/bundlers/generateLibraryPreload.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ export default async function({workspace, taskUtil, options: {skipBundles = [],
});
}
const coreVersion = taskUtil?.getProject("sap.ui.core")?.getVersion();

const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
const execModuleBundlerIfNeeded = ({options, resources}) => {
if (skipBundles.includes(options.bundleDefinition.name)) {
log.verbose(`Skipping generation of bundle ${options.bundleDefinition.name}`);
Expand All @@ -267,6 +267,7 @@ export default async function({workspace, taskUtil, options: {skipBundles = [],
options.targetUi5CoreVersion = coreVersion;
}
options.bundleDefinition = applyDefaultsToBundleDefinition(options.bundleDefinition, taskUtil);
options.allowStringBundling = allowStringBundling;
return moduleBundler({options, resources});
};

Expand Down
3 changes: 3 additions & 0 deletions lib/tasks/bundlers/generateStandaloneAppBundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export default async function({workspace, dependencies, taskUtil, options}) {
});
}

const allowStringBundling = taskUtil?.getProject().getSpecVersion().lt("4.0");
const bundleOptions = {
bundleDefinition: applyDefaultsToBundleDefinition(
getBundleDefinition({
Expand All @@ -149,6 +150,7 @@ export default async function({workspace, dependencies, taskUtil, options}) {
}),
taskUtil
),
allowStringBundling
};

const bundleDbgOptions = {
Expand All @@ -164,6 +166,7 @@ export default async function({workspace, dependencies, taskUtil, options}) {
optimize: false,
},
moduleNameMapping: unoptimizedModuleNameMapping,
allowStringBundling
};

if (coreVersion) {
Expand Down
11 changes: 11 additions & 0 deletions test/expected/build/application.n/dest/Component-dbg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
sap.ui.define(["sap/ui/core/UIComponent", "sap/m/Button", "application/n/MyModuleRequiringGlobalScope"], (UIComponent, Button) => {
"use strict";
return UIComponent.extend("application.n.Component", {
metadata: {
manifest: "json"
},
createContent() {
return new Button({text: magic.text});
}
});
});
2 changes: 2 additions & 0 deletions test/expected/build/application.n/dest/Component.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/expected/build/application.n/dest/Component.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
const magic = {text: "It's magic!"};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions test/expected/build/application.n/dest/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Application N</title>

<script id="sap-ui-bootstrap"
src="resources/sap-ui-core.js"
data-sap-ui-xx-waitForTheme="true"
data-sap-ui-theme="sap_horizon"
data-sap-ui-resourceRoots='{
"application.n": "./"
}'
data-sap-ui-onInit="module:sap/ui/core/ComponentSupport"
data-sap-ui-compatVersion="edge"
data-sap-ui-async="true">
</script>
</head>

<body class="sapUiBody">
<div data-sap-ui-component data-name="application.n" data-id="container" data-settings='{"id" : "n"}'></div>
</body>
</html>
23 changes: 23 additions & 0 deletions test/expected/build/application.n/dest/manifest.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"_version": "1.21.0",
"sap.app": {
"id": "application.n",
"type": "application",
"applicationVersion": {
"version": "1.0.0"
}
},
"sap.ui5": {
"contentDensities": {
"compact": true,
"cozy": true
},
"dependencies": {
"minUI5Version": "1.108.0",
"libs": {
"sap.ui.core": {},
"sap.m": {}
}
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions test/expected/build/application.n/legacy/Component-dbg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
sap.ui.define(["sap/ui/core/UIComponent", "sap/m/Button", "application/n/MyModuleRequiringGlobalScope"], (UIComponent, Button) => {
"use strict";
return UIComponent.extend("application.n.Component", {
metadata: {
manifest: "json"
},
createContent() {
return new Button({text: magic.text});
}
});
});
2 changes: 2 additions & 0 deletions test/expected/build/application.n/legacy/Component.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/expected/build/application.n/legacy/Component.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
const magic = {text: "It's magic!"};
Loading
Loading