fix: CPE loading changes from backend and not from workspace #2429
Conversation
🦋 Changeset detectedLatest commit: dd8a125 The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 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 |
…SAP/open-ux-tools into fix/cpe-loading-backend-changes
| const getNestedProperty = (obj, target) => | ||
| target in obj | ||
| ? obj[target] | ||
| : Object.values(obj).reduce((acc, val) => { | ||
| if (acc !== undefined) return acc; | ||
| if (typeof val === 'object') return getNestedProperty(val, target); | ||
| }, undefined); | ||
|
|
||
| const asyncHints = getNestedProperty(window['sap-ushell-config'], 'asyncHints'); | ||
| if (asyncHints && asyncHints.requests) { | ||
| asyncHints.requests = []; | ||
| } |
There was a problem hiding this comment.
The code here looks more complicated than it should be. First, .reduce() is intended for accumulating values, but here you just need to find and modify a property, this can be simplified.
Your code will continue to check all properties even after finding asyncHints. The .reduce() call may still iterate over other properties in the object, even though the target has already been located. This happens because .reduce() doesn't know that asyncHints has already been found. It will continue checking other values until the entire object is traversed.
Here is the suggestion (since you are not exporting this function anywhere and it is not reusable):
const setRequestsToEmptyArray = (obj) => {
if (!obj || typeof obj !== 'object') return;
for (const key in obj) {
if (key === 'asyncHints' && obj[key]?.requests) {
obj[key].requests = [];
return;
}
setRequestsToEmptyArray(obj[key]);
}
};
setRequestsToEmptyArray(window['sap-ushell-config']);This is easier to read and maintain.
But the complexity here is O(N), where N is the total number of properties across all levels. The function may need to visit every property in the entire object structure if the target property ('asyncHints') is not found until the last node.
What I am trying to say is, if you know the structure and it is constant then you can just do this:
if (
window['sap-ushell-config']?.level1?.level2?.asyncHints?.requests
) {
window['sap-ushell-config'].level1.level2.asyncHints.requests = [];
}And it will be constant time complexity, O(1). Or you can use iterative approaches.
There was a problem hiding this comment.
As discussed, I changed the algorithm used for setting the nested property.
Fixed in: 57805ce
|
| // For UI5 version 1.71 and below, we need to remove the asyncHints.requests to load the changes in an Adaptation project. | ||
| // This logic needs to be executed here to have a reliable check for UI5 version and remove the asyncHints.requests before the sandbox is loaded. | ||
| // The sandbox shell modifies the `window['sap-ushell-config']`. | ||
| if (majorUi5Version === 1 && minorUi5Version < 72) { | ||
| const setNestedProperty = (obj) => { | ||
| if (!obj || typeof obj !== 'object') return; | ||
|
|
||
| const stack = [obj]; | ||
|
|
||
| while (stack.length > 0) { | ||
| const current = stack.pop(); | ||
|
|
||
| if (current.asyncHints) { | ||
| if (current.asyncHints.requests) { | ||
| current.asyncHints.requests = []; | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| for (const key in current) { | ||
| if (typeof current[key] === 'object' && current[key] !== null) { | ||
| stack.push(current[key]); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| setNestedProperty(window['sap-ushell-config']['applications']); | ||
| } |
There was a problem hiding this comment.
Can we please have an own function for this and put the comment about what it does and why it's needed in the respective JSDoc? Including setNestedProperty into the ushellBootstrap function disrupts the readability of the code.
| // For UI5 version 1.71 and below, we need to remove the asyncHints.requests to load the changes in an Adaptation project. | |
| // This logic needs to be executed here to have a reliable check for UI5 version and remove the asyncHints.requests before the sandbox is loaded. | |
| // The sandbox shell modifies the `window['sap-ushell-config']`. | |
| if (majorUi5Version === 1 && minorUi5Version < 72) { | |
| const setNestedProperty = (obj) => { | |
| if (!obj || typeof obj !== 'object') return; | |
| const stack = [obj]; | |
| while (stack.length > 0) { | |
| const current = stack.pop(); | |
| if (current.asyncHints) { | |
| if (current.asyncHints.requests) { | |
| current.asyncHints.requests = []; | |
| } | |
| return; | |
| } | |
| for (const key in current) { | |
| if (typeof current[key] === 'object' && current[key] !== null) { | |
| stack.push(current[key]); | |
| } | |
| } | |
| } | |
| }; | |
| setNestedProperty(window['sap-ushell-config']['applications']); | |
| } | |
| if (majorUi5Version === 1 && minorUi5Version < 72) { | |
| removeAsyncHintsRequests(); | |
| } |
/**
* For UI5 version 1.71 and below, we need to remove the asyncHints.requests
* to load the changes in an Adaptation project.
* This logic needs to be executed here to have a reliable check for
* UI5 version and remove the asyncHints.requests before the sandbox is loaded.
* The sandbox shell modifies the `window['sap-ushell-config']`.
*/
function removeAsyncHintsRequests() {
const obj = window['sap-ushell-config']['applications'];
if (!obj || typeof obj !== 'object') return;
const stack = [obj];
while (stack.length > 0) {
const current = stack.pop();
if (current.asyncHints) {
if (current.asyncHints.requests) {
current.asyncHints.requests = [];
}
return;
}
for (const key in current) {
if (typeof current[key] === 'object' && current[key] !== null) {
stack.push(current[key]);
}
}
}
}
#2445
applicationDependenciesof thesap-ushell-configin the htmlasyncHints.requeststo an empty string for SAPUI5 versions lower than 1.72workspacePathURL parameter to the LREP request to allow for correctly setting the workspace path in case of Adaptation Project