search

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
466 stars 71 forks source link

Preload bundles do not support modules defining global variables #438

Closed kristian closed 3 years ago

kristian commented 5 years ago

In case a module is defining a variables in global scope, such as (only a selection):

sap/ui/thirdparty/blanket.js
sap/ui/thirdparty/flexie.js
sap/ui/thirdparty/hyphenopoly/hyphenEngine.asm.js
sap/ui/thirdparty/mobify-carousel.js
sap/ui/thirdparty/require.js
sap/ui/thirdparty/sinon-ie.js
sap/ui/thirdparty/sinon-server.js
sap/ui/thirdparty/swipe-view.js
sap/ui/thirdparty/zyngascroll.js

Building a preload bundle will result in the variables not be defined on global scope any longer, as the preload generation will wrap the module into a function itself, thus rendering the globally defined vars as function scoped vars instead.

Expected Behavior

Building a preload bundle with a depdendency to e.g. sap/ui/thirdparty/require should still expose requirejs, require and define on global scope, just like loading the application w/o a preload bundle.

Current Behavior

Global variables are not exposed, this could lead to side effects with other modules.

Context

Affected components (if known)

codeworrior commented 5 years ago

After looking deeper into it, this is a bit different than originally expected. The builder already can handle modules with global vars correctly and it contains code to identify them.

But in case raw module info is maintained in .library files, this info supersedes the analysis (which is even skipped then). On the other side, raw-module info was never meant to be complete, it only should enhance the analysis results.

The raw module info has been given priority over the analysis step when support for standard AMD APIs was added (I can't reference the corresponding commit as this was implemented in an internal repo before going open source. But the code is here). The analysis of standard AMD bundles caused issues when the embedded modules are not available to the UI5 tooling. This approach should be revised so that the raw-module info and the analysis results can be merged again. This will fix the issue reported here.

kristian commented 5 years ago

Hey @codeworrior! Thanks for the in-depth analysis and the quick solution, as always! =) Looking forward of this beeing merged!

I tried building with the fix you submitted and got quite a verbose log output (I'll send it to you the logs privately, as they contain some internal information). Hope this helps! Thanks again!