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

Preload bundles do not support modules defining global variables #438

Closed
1 of 6 tasks
kristian opened this issue Sep 20, 2019 · 2 comments · Fixed by SAP/ui5-builder#340
Closed
1 of 6 tasks

Preload bundles do not support modules defining global variables #438

kristian opened this issue Sep 20, 2019 · 2 comments · Fixed by SAP/ui5-builder#340
Assignees
Labels
module/ui5-builder Related to the UI5 Builder module

Comments

@kristian
Copy link

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

  • UI5 Module Version: 1.5.1

Affected components (if known)

@codeworrior codeworrior self-assigned this Sep 22, 2019
@codeworrior
Copy link
Member

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
Copy link
Author

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!

codeworrior referenced this issue in SAP/ui5-builder Oct 11, 2019
…os (#340)

[FIX] Bundling: merge dependency analysis results with raw module infos

Fixes https://github.com/SAP/ui5-builder/issues/338

Also adds tests for Resolver
@RandomByte RandomByte transferred this issue from SAP/ui5-builder Nov 20, 2020
@RandomByte RandomByte added the module/ui5-builder Related to the UI5 Builder module label Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/ui5-builder Related to the UI5 Builder module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants