[FIX] manifestCreator: 'embeds' should list all components #575
Conversation
embeds field in created manifest.json now contains all nested manifests independent from the reverse field embeddedBy, which points from an embedded manifest to the library's manifest.
Removed invalid code in sapui5 version section (#getUI5Version) Fixed version in sections: use a version string instead of Semver version object
not require it
not require it
| let ui5Version; | ||
| if ( ui5Version != null ) { | ||
| return ui5Version; | ||
| } |
There was a problem hiding this comment.
I wonder where this code came from. It doesn't make sense, but maybe it needs to be fixed rather than being removed.
There was a problem hiding this comment.
The code taken from the Java implementation had a member variable ui5version which could be set from the oustide via parameter. This is not the case here anymore so it can be removed.
| }; | ||
| t.is(errorLogStub.callCount, 0); | ||
|
|
||
| const options = {descriptorVersion: new Version("1.1.0")}; |
There was a problem hiding this comment.
I wonder why the API requires a semver version instance instead of a plain string.
AFAICS the whole processor is not marked as public API so we should be able to do a cleanup.
As this is not related to the actual PR of fixing the embeds behavior, I'd be in favor of creating a separate PR.
Or is there a relation from the version handling change (toString) to the embeds change? At least I could not find any hint. Maybe to increase code coverage?
There was a problem hiding this comment.
inside the manifestCreator the SemVer object is used for version comparison, as api parameter (descriptorVersion) a string definitely makes sense and can be done in a separate PR.
This test/fix was made to increase the test coverage. During a normal run such a small version is not used.
There was a problem hiding this comment.
Also see #556 where we already discussed making the manifestCreator public and that we would re-check the API before doing so.
| const result = []; | ||
| const prefix = libraryResource.getPath().slice(0, - ".library".length); | ||
| const prefix = posixPath.dirname(libraryResource.getPath()) + "/"; |
The descriptorVersion parameter is not public API, but it makes sense to make the parameter of type string and create a SemVer object inside.
matz3
changed the title
Field
embedsin created manifest.json file now contains all nestedmanifests, independent from the reverse field
embeddedBy(embeddedBypoints from an embedded manifest to the library's manifest).
Remove unreachable code (in
createSapUI5()->getUI5Version()function) .Use
path.dirnamemethod to get directory name infindEmbeddedComponents()function.Use string for version value in manifest (in
sectionVersion()function).e.g. "1.2.3" instead of the semver object's string representation.
It reverts changes which were introduced with #555
Successor of #554