perf: Improve rendering performance for large configuration models [CXSPA-7477]

[Uli-Tiger]

Issue: The product configuration UI is completely re-rendered after each UI interaction. This may lead to performance issues for large configuration models, where a lot of attributes (>50) and/or a lot of possible values per attribute (>50) are rendered on the UI.

Solution: When this feature toggle(productConfigurationDeltaRendering) is activated, only these parts of the UI are re-rendered, that actually changed, significantly (up to factor 10) improving rendering performance for large models. Please note, this will influence how the pricing requests are processed and rendered. Instead of merging the prices into the configuration model, which effectively triggers re-rendering the whole UI-Component tree, the price supplements are kept in a separate subtree of the model, so that UI components can react independently on pricing chnages
Hence, it is advised to do full regression testing after activation of this flag and before rolling this out to production.

Details: This optimization consists of 3 main parts:
(1) A smart change detection for attributes implemented in configurator-attribute-composition.directive.ts. Basically, the directive will compare last rendered attribute state with the current attribute state and only trigger re-rendering of the attribute if the content of the attribute changed. Additionally, the ngFor generating the attribute list is enhanced with a trackBy function based on the attribute key.
(2) The ngFor generating the group menu is enhanced with a trackBy function based on the group id.
(3) Decouple pricing updates from configuration changes. Instead of merging pricing changes, which are fetched asynchronously, into the leaves (values) of the configuration tree within the configuration.reducer, the pricing changes are kept in a separate subtree configuration.priceSupplements. Unfortunately any attribute component and not only the pricing component should react on pricing changes. The attribute components build the ARIA labels for the attributes including any price related information (surcharges). To help attribute components to track pricing changes a new component service ConfiguratorDeltaRenderingService has been introduced. It monitors the configuration observable for pricing supplements fitting the given attribute key. Its rerender method will emit whenever a pricing change was detected. So the prices can now be updated independently from the rest of the configuration model. This is also a prerequisite for the optimization (1), as otherwise fetching the prices would always trigger re-rendering of all attributes.

[Platonn]

[MINOR]
Do we really have to call this.initPriceChangedEvent() it in every child class?

Can't we inject(ConfiguratorAttributeCompositionContext) in the super class ConfiguratorAttributeBaseComponent? and call this.initPriceChangedEvent() there? - so it will be self-sufficient?

Thanks to this, the child classes would be simpler, would you agree?

[Uli-Tiger]

inject(ConfiguratorAttributeCompositionContext) is only injected for components that can occur when configuring with CPS, but not to CPQ exclusive components and this was already done/decided before this PR, so i would like to avoid to refactor this now.

[Platonn]

[MEDIUM] The name of it is a bit misleading.

• A the first sight I pretty much thought it's a kinda "setter" -> setting the given data in the state of the service. But after reading the implementation it carefully, I realised it's a kinda "getter" and has no side effects.
• it's not clear whether where the price is coming from (from the given Value param?). I understand that not. But it's a rather currentPrice that is known only by the service.

For easier reasoning about this method (without needing to check its implementation), what do you think about renaming it to something like get....? e.g. getWithCurrentPrice(value: Configurator.Value): Configurator.Value?

or alternatively just have simpler method that doesn't perform merging, but only returns the price, e.g. getCurrentPrice(valueName: string): Configurator.PriceDetails - it would be much easier for me to reason about. Then I clearly know that this method doesn't affect the state of the service.

[Platonn]

UPDATE:
[MAJOR] (my late reflection after digesting the big picture of this service)

We're mixing a reactive programing (Rxjs observable flow in getPriceChangedEvents()) with static properties (accessing data statically via mergePriceIntoValue which were stored as a side effect of observable flow ofgetPriceChangedEvents()), with is generally hard to reason about and btw. error prone in async edge cases.

Ideally, I'd suggest that we should rather have no static getter methods like mergePriceIntoValue for our reactive data, but instead emit from our returned Observable all the the data needed by the consumers e.g.:

- getPriceChangedEvents(attributeKey: string | undefined): Observable<boolean> {
+ getChangedPrices(attributeKey: string | undefined): Observable<
+   { pricesByValueName: Record<string, Configurator.PriceDetails }
+ > {
    /* ... */
-               return pricesChanged ? of(true) : EMPTY;
+               return pricesChanged ? of({ this.pricesByValueName }) : EMPTY;

Then in the consuming component we could have:

changedPrices$ = of({ pricesByValueName: {} });
/* ... */
this.changedPrices$ = this.service.getChangedPrices(/*...*/)

and then use it in HTML template e.g.:

<ng-container *ngIf="(changedPrices$ | async) as changedPrices">
   <!-- ... -->
   <option
     [label]="getLabel(
                expMode, 
                item.valueDisplay, 
                item.valueCode, 
                this.enrichValueWithPrice(item, changedPrices.pricesByValueName[item.name])
              )"

And we would need to expose an util method in the ConfiguratorAttributeBaseComponent so it's accessible int he HTML template:

enrichValueWithPrice(value: Configurator.Value, price: Configurator.PriceDetails): Configurator.Value {
  if (price) {
    value = { ...value, valuePrice: price };
  }
  return value;
}

What do you think about it?

[Uli-Tiger]

will be covered in: #19097