Skip to content

[FEATURE] Generate source maps for bundles#695

Merged
RandomByte merged 33 commits intonextfrom
feature-bundle-source-maps
Feb 23, 2022
Merged

[FEATURE] Generate source maps for bundles#695
RandomByte merged 33 commits intonextfrom
feature-bundle-source-maps

Conversation

@RandomByte
Copy link
Member

@RandomByte RandomByte commented Jan 26, 2022

Supersedes #282

Based on UI5/cli#583

JIRA: CPOUI5FOUNDATION-434

@RandomByte RandomByte force-pushed the feature-bundle-source-maps branch from 863a79d to 7303498 Compare January 26, 2022 16:30
@coveralls
Copy link

coveralls commented Feb 8, 2022

Coverage Status

Coverage decreased (-0.1%) to 94.672% when pulling a6e2f06 on feature-bundle-source-maps into 3b51c1b on next.

@RandomByte RandomByte force-pushed the feature-bundle-source-maps branch from 4930295 to 7b51d11 Compare February 14, 2022 10:42
@RandomByte RandomByte marked this pull request as ready for review February 22, 2022 15:01
@matz3 matz3 force-pushed the feature-bundle-source-maps branch from e74779f to 9bb1025 Compare February 22, 2022 15:10
@RandomByte RandomByte force-pushed the feature-bundle-source-maps branch from ae415dc to c13ce14 Compare February 23, 2022 16:53
@RandomByte RandomByte merged commit 8a20c42 into next Feb 23, 2022
@RandomByte RandomByte deleted the feature-bundle-source-maps branch February 23, 2022 17:35
* @typedef {object} ModuleBundleOptions
* @property {boolean} [optimize=true] Whether the module bundle gets minified
* @property {boolean} [sourceMap] Whether to generate a source map file for the bundle.
* Defaults to true if <code>optimize</code> is set to true
Copy link
Member

Choose a reason for hiding this comment

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

This is not correct anymore. A sourcemap is always created unless sourceMap: false is set.

In addition: Is this something to be added to the project configuration schema?

Copy link
Member

Choose a reason for hiding this comment

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

The default will be updated via #709

Copy link
Member

Choose a reason for hiding this comment

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

I've added a bullet-point for adding 'sourceMap' to the specVersion 3.0 bundleOptions to UI5/cli#506

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, adding this to the bundleOptions makes sense

* @param {ModuleBundleDefinition} parameters.options.bundleDefinition Module bundle definition
* @param {ModuleBundleOptions} [parameters.options.bundleOptions] Module bundle options
* @returns {Promise<module:@ui5/fs.Resource[]>} Promise resolving with module bundle resources
* @returns {Promise<module:@ui5/builder.processors.MinifierResult[]>} Promise resolving with module bundle resources
Copy link
Member

Choose a reason for hiding this comment

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

ModuleBundlerResult

Copy link
Member

Choose a reason for hiding this comment

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

Will be solved via #709

* @alias module:@ui5/builder.processors.minifier
* @param {object} parameters Parameters
* @param {module:@ui5/fs.Resource[]} parameters.resources List of resources to be processed
* @param {boolean} [parameters.addSourceMappingUrl=true]
Copy link
Member

Choose a reason for hiding this comment

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

Usually in our processors we have an options object that contains such parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I forgot about that. This should be changed then 👍

Copy link
Member

Choose a reason for hiding this comment

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

Fixed via #709

.then((processedResources) => {
return Promise.all(processedResources.map((resource) => {
.then((results) => {
const bundles = Array.prototype.concat.apply([], results);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this is needed in here

Copy link
Member Author

Choose a reason for hiding this comment

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

I wonder the same 🤷

Comment on lines +115 to +130
// For "unoptimized" bundles, the non-debug files have already been filtered out above.
// Now we need to create a mapping from the debug-variant resource path to the respective module name,
// which is basically the non-debug resource path, minus the "/resources/"" prefix.
// This mapping overwrites internal logic of the LocatorResourcePool which would otherwise determine
// the module name from the resource path, which would contain "-dbg" in this case. That would be
// incorrect since debug-variants should still keep the original module name.
for (let i = unoptimizedResources.length - 1; i >= 0; i--) {
const resourcePath = unoptimizedResources[i].getPath();
if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) {
const nonDbgPath = ModuleName.getNonDebugName(resourcePath);
if (!nonDbgPath) {
throw new Error(`Failed to resolve non-debug name for ${resourcePath}`);
}
unoptimizedModuleNameMapping[resourcePath] = nonDbgPath.slice("/resources/".length);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

We have this code in three places now, so it would be better to refactor it

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, maybe we could have all bundling tasks call the generateBundle-task or create a "helper" module in lib/tasks/bundlers/ for this purpose 👍

This was referenced Mar 8, 2022
flovogt added a commit that referenced this pull request Mar 9, 2022
@flovogt flovogt mentioned this pull request Mar 30, 2022
This was referenced May 4, 2022
d3xter666 pushed a commit to UI5/cli that referenced this pull request Sep 25, 2025
)

Resolves #472
Supersedes SAP/ui5-builder#282
Based on #583
JIRA: CPOUI5FOUNDATION-434

Co-authored-by: Matthias Osswald <mat.osswald@sap.com>
d3xter666 pushed a commit to UI5/cli that referenced this pull request Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants