[FIX] Improve recognition of main module in case of bundles #341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a port of the corresponding changes from the Maven tooling,
which have been made after the migration of the bundle tooling.
See (SAP internal) changes
3101244 adapt merge tooling to the renaming of ui5loader.js
3312369 improve recognition of main module in case of bundles
3945349 JSModuleAnalyzer fails on modules that don't declare a name
and 3562114 fix dep. analysis of new evo bundles
Commit 1: migrate tests for TDD
[FIX] Infra: improve recognition of main module in case of bundles
So far, the module analyzer always assumed the first module definition
to be the main definition. This failed at least for CVOM bundles but
might also fail for others.
The analysis has been improved and now accepts a module as main module
only in the following cases:
- if it is an unnamed module
- if it is a named module whose name matches the external name
- if it is a named module and there's only one module (no bundle)
If multiple module definitions match the above criteria, this is
reported as an error.
Additionally, AMD special dependencies ('require', 'exports', 'module')
are no longer reported in the dependency info.
This fixes all test, except the EVO bundle test.
FIX] Infra: JSModuleAnalyzer fails on modules that don't declare a name Basically, the JavaScript implementation was less sensitive to this issue. But if a resource could have a dependency to another resource named "null", the JavaScript tooling would fail, too. Although this looked like a purely theoretical scenario, adding a protective 'if' was trivial enough to just do it.
[INTERNAL] Infra: fix dep. analysis of new evo bundles - recognize and evaluate sap.ui.require.preload calls - write and extract bundle name and raw module names
codeworrior
changed the title
matz3
approved these changes
Sep 30, 2019
|
I intended to squash them. That's why I already updated the initial comment with a 'squashed' commit message. |
|
👍 |
3 tasks
matz3
added a commit
that referenced
this pull request
Feb 5, 2020
The UI5 core library contains dynamic calls to sap.ui.require.preload. Those calls should not result into a warning in the build log, as they are correct and should just be skipped. This has been introducted with #341.
3 tasks
matz3
added a commit
that referenced
this pull request
Feb 6, 2020
The UI5 core library contains dynamic calls to sap.ui.require.preload. Those calls should not result into a warning in the build log, as they are correct and should just be skipped. This has been introducted with #341.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a port of several changes from the Maven tooling, which have been made after the initial migration of the bundle tooling. Additionally, code coverage has been slightly improved by adding additional unit and integration tests. Errors that have been revealed by the new tests have been fixed.
Details about the changes:
Change 3312369 (improve recognition of main module in case of bundles):
So far, the module analyzer always assumed the first module definition to be the main definition. This failed at least for CVOM bundles but might also fail for others.
The analysis has been improved and now accepts a module as main module only in the following cases:
If multiple module definitions match the above criteria, this is
reported as an error.
Additionally, AMD special dependencies ('require', 'exports', 'module') are no longer reported in the dependency info.
Change 3945349 (JSModuleAnalyzer fails on modules that don't declare a name)
Basically, the JavaScript implementation was less sensitive to this issue. But if a resource could have a dependency to another resource named "null", the JavaScript tooling would fail, too. Although this looked like a purely theoretical scenario, adding a protective 'if' was trivial enough to just do it.
Change 3562114 (fix dep. analysis of new evo bundles)