[FEATURE] Add minify task and processor by RandomByte · Pull Request #666 · SAP/ui5-builder

RandomByte · 2021-11-23T17:14:42Z

Implements UI5/cli#583 based on #657

Depends on SAP/ui5-fs#331

Combine debug-file creation, minification and source-map generation into a single minify task
- Replaces existing uglify and createDebugFiles tasks
Tag debug-files with new "global" (=shared across projects) tag IsDebugVariant
Filter debug-files from moduleBundler so that it only packages minified or debug-sources (depending on optimize bundleOption)

Open TODOs:

Add integration test (or modify an existing one) for optimize: true bundle generation without minify task
Add integration test (or modify an existing one) for optimize: false bundle generation with minify task

test/lib/index.js

RandomByte · 2021-12-23T12:41:30Z

Tests are failing because of dependency to SAP/ui5-fs#331

matz3 · 2021-12-23T12:21:45Z

lib/lbt/resources/LocatorResourcePool.js

 const LocatorResource = require("./LocatorResource");

 class LocatorResourcePool extends ResourcePool {
+	constructor({ignoreMissingModules} = {}) {


Unnecessary change, right?

matz3 · 2021-12-23T12:22:54Z

lib/lbt/resources/ResourcePool.js


 async function determineDependencyInfo(resource, rawInfo, pool) {
-	const info = new ModuleInfo(resource.name);
+	const info = new ModuleInfo(ModuleName.getNonDebugName(resource.name) || resource.name);


Do we still need this?

matz3 · 2021-12-23T12:25:14Z

lib/processors/minifier.js

+		dbgResource.setPath(dbgPath);
+
+		const filename = path.posix.basename(resource.getPath());
+		if (filename.endsWith(".support.js")) {


AFAIK this is the reason why it should not be done

Suggested change

if (filename.endsWith(".support.js")) {

// Support rules should not be minified to have readable code in the Support Assistant

if (filename.endsWith(".support.js")) {

Done, moved to glob pattern in Application- and LibraryBuilder

matz3 · 2021-12-23T12:28:02Z

lib/tasks/minify.js

+
+	return Promise.all(processedResources.map(async ({resource, dbgResource, sourceMapResource}) => {
+		if (taskUtil) {
+			taskUtil.setTag(resource, taskUtil.STANDARD_TAGS.HasDebugVariant);


Is this really correct for the "*.support.js" files?
I would expect that those should be skipped completely (no tags set).

matz3 · 2021-12-23T12:30:17Z

test/lib/processors/minifier.js

+	t.deepEqual(await resources[3].resource.getPath(), "/test4.support.js",
+		"Correct resource path for minified content of resource 4");
+	t.deepEqual(await resources[3].resource.getString(), expectedMinified4, "Correct minified content for resource 4");
+	t.deepEqual(await resources[3].dbgResource.getPath(), "/test4-dbg.support.js",


This assertion should fail, right?

Not relevant anymore, minifier now has no special handling of support files.

matz3 · 2021-12-23T12:30:36Z

test/lib/processors/minifier.js

+		`{"version":3,"sources":["test3-dbg.designtime.js"],"names":["test3","paramA","variableA","console","log"],` +
+		`"mappings":"AACA,SAASA,MAAMC,GACd,IAAIC,EAAYD,EAChBE,QAAQC,IAAIF,GAEbF","file":"test3.designtime.js"}`;
+
+	t.deepEqual(await resources[0].resource.getPath(), "/test1.controller.js",


getPath isn't async, right?

matz3 · 2021-12-23T12:34:44Z

lib/tasks/bundlers/generateLibraryPreload.js

+					}).transformer(async function(resourcePath, getClonedResource) {
+						if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) {
+							const resource = await getClonedResource();


Suggested change

}).transformer(async function(resourcePath, getClonedResource) {

if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) {

const resource = await getClonedResource();

}).transformer(async function(resourcePath, getResource) {

if (taskUtil.getTag(resourcePath, taskUtil.STANDARD_TAGS.IsDebugVariant)) {

const resource = await getResource();

matz3 · 2021-12-23T12:41:39Z

test/expected/build/application.i/dest/Component-preload.js

 "version":"2.0",
 "modules":{
 	"application/i/Component.js":function(){sap.ui.define(["sap/ui/core/UIComponent"],function(n){"use strict";return n.extend("application.i.Component",{metadata:{manifest:"json"}})});
+//# sourceMappingURL=Component.js.map


Should those comments be part of the preload? If the browser ignores them I guess it's okay. If it causes problems, we should remove them.

I think they are not a problem unless they are at the end of the file. Anyways, the source map change for the module bundler will remove them 👍

matz3 · 2021-12-23T12:43:24Z

test/expected/build/library.ø/dest/resources/library/ø/rules/MyControl-dbg.support.js

As already mentioned above: We don't want to create -dbg files for *.support.js files. So they should just remain as-is.

…ebugVariant'

Combines debug-file creation, source-map generation and minification. Replaces existing tasks/processors 'uglify' and 'createDebugFiles'. BREAKING CHANGE: The following tasks have been removed: - createDebugFiles - uglify The following processors have been removed: - debugFileCreator - resourceCopier - uglifier As a replacement, the new 'minify' task and 'minifier' processors can be used. Note: The minify task is now executed earlier, before the bundling process takes place. The 'beforeTask' and 'afterTask' configuration of custom tasks might need to be adapted to cater for this change.

Bundling can now rely on preceding minify task

…ix error message Do not check whether a debug-variant already exists. Just overwrite it

…ifier

See SAP/ui5-fs#331

…ith and without minify task

…ce names

Only for bundles configured as 'optimize: false'. Removes LocatorResourcePool option to remove -dbg from resource names.

…unoptimized sap.ui.core bundles Only applicable to sap.ui.core versions <1.97.0. Same logic as for custom bundles in the generateBundle task.

Without building all dependencies, some modules might not be minified now

…ved tasks

…uilder instead of explicit implementation

coveralls · 2022-01-14T08:33:21Z

Coverage increased (+0.01%) to 94.782% when pulling c0676d6 on minify-task into c110dbc on next.

RandomByte · 2022-01-14T08:38:58Z

Rebased on top of latest next to get new ui5-fs features

lib/tasks/TaskUtil.js

matz3 · 2022-01-14T14:01:32Z

lib/tasks/TaskUtil.js

+		return this._projectBuildContext.getResourceTagCollection().clearTag(resourcePath, tag);
+	}
+
+	getResourceTagCollection() {


But.. is this method actually used somewhere?
I couldn't find a reference.

Right, this shouldn't be necessary anymore. Probably it was added for some different, intermediate implementation.

matz3 · 2022-01-14T14:06:05Z

lib/tasks/bundlers/generateStandaloneAppBundle.js

 			`unable to generate complete bundles for such projects.`);
 	}

-	// If an application does not have a namespace, its resources are located at the root. Otherwise in /resources


By removing this behavior, we need to make sure to fail in case we don't have a namespace (as already planned for 3.0)

Yes, correct!

Combines debug-file creation, source-map generation and minification. Replaces existing tasks/processors 'uglify' and 'createDebugFiles'. Also introduce new global tags `IsDebugVariant` and `HasDebugVariant`. BREAKING CHANGE: The following tasks have been removed: - createDebugFiles - uglify The following processors have been removed: - debugFileCreator - resourceCopier - uglifier As a replacement, the new 'minify' task and 'minifier' processor can be used. Note: The minify task is executed earlier, before the bundling process takes place. Existing 'beforeTask' and 'afterTask' configuration of custom tasks might need to be adapted to cater for this change.

RandomByte force-pushed the minify-task branch 2 times, most recently from 0877d46 to 167af6b Compare November 25, 2021 12:27

matz3 requested changes Nov 29, 2021

View reviewed changes

test/lib/index.js Outdated Show resolved Hide resolved

RandomByte marked this pull request as ready for review December 3, 2021 08:51

RandomByte force-pushed the minify-task branch 2 times, most recently from fb813f6 to 9b5eddf Compare December 22, 2021 16:18

matz3 requested changes Dec 23, 2021

View reviewed changes

RandomByte force-pushed the minify-task branch 2 times, most recently from 8a71cde to a6ad03a Compare December 23, 2021 14:52

RandomByte added 20 commits January 14, 2022 09:31

[FEATURE] BuildContext: Introduce global tags 'IsDebugVariant', 'HasD…

1ea1bb7

…ebugVariant'

[INTERNAL] Bundle generation: Filter (non-)debug resources

90d95bc

[INTERNAL] lbt/bundle/Builder: Remove compression

a4d31f9

Bundling can now rely on preceding minify task

[INTERNAL] minifier: Preserve 'sap' variables during mangling

c0d4c30

[INTERNAL] minifier: Skip debug-variant check, rename return value, f…

705c54c

…ix error message Do not check whether a debug-variant already exists. Just overwrite it

[INTERNAL] minifier: Merge createDebugFiles and uglify tests into min…

e14933b

…ifier

[INTERNAL] Update tests

d780860

[INTERNAL] Update way too many expected files

36b6e98

[INTERNAL] BuildContext: Update tests

7885d19

[INTERNAL] Bundle generators: Adapt to latest ReaderFilter changes

dcc6047

See SAP/ui5-fs#331

[INTERNAL] builder integration tests: Add tests for optimize: false w…

801b508

…ith and without minify task

[INTERNAL] generateStandaloneAppBundle: Add filtering of -dbg variants

4003b79

[INTERNAL] LocatorResourcePool: Add option to remove -dbg from resour…

f91f11d

…ce names

[INTERNAL] Use ReaderTransformer to remove -dbg from resource names

6c432d6

Only for bundles configured as 'optimize: false'. Removes LocatorResourcePool option to remove -dbg from resource names.

[INTERNAL] minifier: Do not minify .support.js resources

ff1b7e6

[INTERNAL] generateBundle: Make transformer callback sync

9562ef9

[INTERNAL] TaskUtil: Allow resource path instead of instance

a6e7b64

[INTERNAL] generateLibraryPreload: Apply resource transformation for …

59dc7bd

…unoptimized sap.ui.core bundles Only applicable to sap.ui.core versions <1.97.0. Same logic as for custom bundles in the generateBundle task.

[INTERNAL] generateLibraryPreload: Update sap.ui.core integration test

9ea9288

RandomByte added 4 commits January 14, 2022 09:31

[INTERNAL] generateStandaloneAppBundle: Update integration test

ca7423f

Without building all dependencies, some modules might not be minified now

[INTERNAL] taskRepository: Helpful error message when retrieving remo…

cc3553e

…ved tasks

[INTERNAL] Cleanup based on code review

c32c5a8

[INTERNAL] Minifer: Exclude .support.js files using glob pattern in b…

c6c1e84

…uilder instead of explicit implementation

RandomByte force-pushed the minify-task branch from a6ad03a to c6c1e84 Compare January 14, 2022 08:31

matz3 reviewed Jan 14, 2022

View reviewed changes

[INTERNAL] TaskUtil: Remove unused function getResourceTagCollection

c0676d6

matz3 approved these changes Jan 17, 2022

View reviewed changes

RandomByte changed the title ~~[FEATURE] Add minify task~~ [FEATURE] Add minify task and processor Jan 17, 2022

RandomByte merged commit a3af604 into next Jan 17, 2022

RandomByte deleted the minify-task branch January 17, 2022 09:41

This was referenced Jan 25, 2022

Custom bundles should not contain unwanted debug-resources UI5/cli#513

Closed

[PoC] Refactor bundle process to exclude debug-files and enable source maps #657

Closed

[RFC] 00010 UI5 Builder-Bundling Refactoring UI5/cli#583

Merged

This was referenced Mar 8, 2022

fix locator resource pool #717

Closed

next #718

Closed

matz3 mentioned this pull request Mar 10, 2022

[INTERNAL] replaceVersion: Also process css files #720

Merged

This was referenced Mar 30, 2022

css variables api next #727

Closed

[FEATURE] generateThemeDesignerResources task: Create css_variables.less #730

Merged

This was referenced May 4, 2022

feat/next/jsdocDemoLink #742

Closed

execution time measurement #744

Closed

matz3 mentioned this pull request Jul 29, 2022

[FEATURE] builder: Improve support for ES6+ syntax #774

Merged

	if (filename.endsWith(".support.js")) {
	// Support rules should not be minified to have readable code in the Support Assistant
	if (filename.endsWith(".support.js")) {

Conversation

RandomByte commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Open TODOs:

Uh oh!

Uh oh!

RandomByte commented Dec 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jan 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RandomByte commented Jan 14, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

RandomByte commented Nov 23, 2021 •

edited

Loading

coveralls commented Jan 14, 2022 •

edited

Loading