SAP / ui5-tooling

An open and modular toolchain to develop state of the art applications based on the UI5 framework
https://sap.github.io/ui5-tooling
Apache License 2.0
468 stars 71 forks source link

Exclude option in build configuration being ignored #358

Closed BenReim closed 1 year ago

BenReim commented 4 years ago

Expected Behavior

Shall not include excluded resources in build.

Current Behavior

Although excluded in configuration, resource is still being considered during build process.

Steps to reproduce the issue

  1. Clone and checkout minimal sample project git clone -b bundle-conf https://github.com/BenReim/openui5-sample-app.git

  2. Run build npm install && ui5 build --a

  3. Watch build log output referring to excluded resource WARN lbt:bundle:Builder **** warning: module sap/ui/demo/todo/scripts/mockserver.js requires top level scope and can only be embedded as a string (requires 'eval')

Log Output / Stack Trace

> ui5 build --a
info builder:builder Building project openui5-sample-app including dependencies...
info builder:builder �   (1/1) Building project openui5-sample-app
info builder:builder application openui5-sample-app � (1/9) Running task escapeNonAsciiCharacters...
info builder:builder application openui5-sample-app � (2/9) Running task replaceCopyright...
info builder:builder application openui5-sample-app � (3/9) Running task replaceVersion...
info builder:builder application openui5-sample-app � (4/9) Running task generateFlexChangesBundle...
info builder:builder application openui5-sample-app � (5/9) Running task generateComponentPreload...
WARN lbt:bundle:Builder **** warning: module sap/ui/demo/todo/scripts/mockserver.js requires top level scope and can only be embedded as a string (requires 'eval')
info builder:builder application openui5-sample-app � (6/9) Running task generateBundle...
info builder:builder application openui5-sample-app � (7/9) Running task createDebugFiles...
info builder:builder application openui5-sample-app � (8/9) Running task uglify...
info builder:builder application openui5-sample-app � (9/9) Running task generateVersionInfo...
info builder:builder Build succeeded in 604 ms
info builder:builder Executing cleanup tasks...
codeworrior commented 4 years ago

The resources/excludes operates on the full scope of project resources while the bundle section filters only work on production resources. The overall exclude therefore rather should be something like

builder:
  resources:
    excludes:
      - "/resources/sap/ui/demo/todo/scripts/**"

But even with that, I still see the message :-(

BenReim commented 4 years ago

Verified that the issue still persists in tooling version 2

BenReim commented 4 years ago

Hope I don’t come across as impatient, but is there any chance this issue will be addressed within the upcoming releases? AFAIK the builder/exclude section just doesn’t work as explained in the documentation.

We would like to use the self-contained build and benefit from a tailored bundle, but can’t do so currently as mock and test files blow up the build result :(

BenReim commented 4 years ago

Hm, just observed that while an absolute path does not work, a relative path does actually work:

builder:
  resources:
    excludes:
      - "/scripts/**"
RandomByte commented 4 years ago

Hey, your exclude needs to be for the runtime path of the project as served by ui5-server. Opposing to @codeworrior comment, for projects of type application this means /scripts/** or /index.html since applications are always served on the root / and without their namespace.

We decided to align this with the Path Mapping of projects.

However, during ui5 build we transform the given exclude /scripts/** to the "virtual base path" /resources/sap/ui/demo/todo/scripts/** for further processing. In your case this led to the path /resources/sap/ui/demo/todo/resources/sap/ui/demo/todo/scripts/**.

We could check wether a given exclude already contains the full virtual base path and skip adding it again. This would allow for both styles. But I'm not sure whether this will make it easier or add more confusion.

BenReim commented 4 years ago

Hey - thanks for the explanation. It definitely makes sense now. 👍

IMHO such a check is not necessary. Though maybe the documentation could be enhanced a bit? I was expecting the use the full virtual paths as the samples in for the builder/resources/excludes option are all starting from /resources/.... The docu terms used (virtual, runtime paths) were not immediately clear too me.

Thanks for your work and all of the exiting features and enhancements that were brought with the tooling version 2! :)

RandomByte commented 1 year ago

Updated the documentation at https://sap.github.io/ui5-tooling/stable/pages/Configuration/#exclude-resources