-
Notifications
You must be signed in to change notification settings - Fork 54
fix(preview-middleware): default flp.libs parameter to true #3774
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
base: main
Are you sure you want to change the base?
fix(preview-middleware): default flp.libs parameter to true #3774
Conversation
🦋 Changeset detectedLatest commit: cf88b83 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…lp.libs-to-true-for-edmx-backends' into fix/preview-middleware/default-flp.libs-to-true-for-edmx-backends
…lp.libs-to-true-for-edmx-backends' into fix/preview-middleware/default-flp.libs-to-true-for-edmx-backends
…rue-for-edmx-backends
…lp.libs-to-true-for-edmx-backends' into fix/preview-middleware/default-flp.libs-to-true-for-edmx-backends
…rue-for-edmx-backends
…lient and preview-middleware
…x-backends' of github.com:SAP/open-ux-tools into fix/preview-middleware/default-flp.libs-to-true-for-edmx-backends
tobiasqueck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR contains quite a lot of changes but in the end you just set the default to true for edmx and added a workaround to the serve static middleware. The rest is code restructurings and cleanups, correct?
That mix made the review a little more time consuming but in the end, the code is now cleaner and better structured. The functional change is also a good idea and should have been done a long time ago (I missed it when creating the initial version of this code). The workaround in the serve static middleware will cause confusion in the future - I am sure of it - but I understand it is required to keep existing setups working as-is.
|
@tobiasqueck it also fixes an issue that the list of UI5 libs is not static but depending on the actual UI5 version. So we now load it from the UI5 version and only use a static list as fallback. Basically this fixes parts of #2683 |
devinea
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @heimwege. Changes look good.
Haven't tested locally.
changeset ✅
…middleware/default-flp.libs-to-true-for-edmx-backends # Conflicts: # packages/preview-middleware/README.md
…rue-for-edmx-backends
…rue-for-edmx-backends
…rue-for-edmx-backends
…rue-for-edmx-backends
|



flp.libsparameter ofpreview-middlewaretotruefor edmx backendsflp.libsparameter ofpreview-middlewaretofalsefor capserve-static-middlewareto additionally serve paths that start with/resourcesw/o the/resourcespart (excluding/resourcesas standalone path) to ensure that the workaround to use servestatic on/resources/my/custom/libbecause theflp.libsparameter was not known/used will still continue to work.Sample: An existing
pathconfiguration/resources/my/custom/libto serve a custom library locally w/oflp.libstruewill have two servestatic paths./resources/my/custom/lib<-- the existing one remains unchanged, but will no longer be used because/my/custom/lib<-- the newflp.libsdefaulttruewill add the custom library and its respective ABAP backend url to the UI5 loader configuration; That's why we add thispathconfiguration on-the-fly to ensure compatibility with the newflp.libstruedefaultSample: An existing
pathconfiguration/resourceswill remain unchanged and still only has one servestatic path so that the whole UI5 framework can still be served locally/resources<-- the existing one remains unchanged, and will continue to work