Adding multiple plugins with wrapComponents on the same core component seems broken

[DayS]

Q&A (please complete the following information)

• OS: macOS
• Browser: Chrome
• Version: 90.0.4430.93
• Method of installation: npm
• Swagger-UI version: [e.g. 3.47.1]
• Swagger/OpenAPI version: OpenAPI 3.0

Content & configuration

Swagger/OpenAPI definition is not relevant for this issue bug here is a sample:

openapi: "3.0.1"
paths:
  /my-endpoint:
    get:
      operationId: getMyAwesomeEndpoint
      responses:
        "200":
          description: "Successful operation"

Swagger-UI configuration options:

import SwaggerUI from 'swagger-ui'
SwaggerUI({
  spec: spec,
  deepLinking: true,
  dom_id: '#swagger',
  plugins: [
    {
      wrapComponents: {
        info: (Original, { React, getComponents }) => (props) => {
          return React.createElement("div", null, [
            React.createElement("h3", {key: 1}, "Hello world! I am above the Info component"),
            React.createElement(Original, Object.assign({key: 2}, props)),
          ])
        }
      }
    },
    {
      wrapComponents: {
        info: (Original, { React, getConfig }) => (props) => {
          return React.createElement("div", null, [
            React.createElement(Original, Object.assign({key: 2}, props)),
            React.createElement("h4", {key: 1}, "Hello world! I am below the Info component") ,
          ])
        }
      }
    }
  ]
});

Describe the bug you're encountering

When multiple plugins are configured with wrapComponents entry on the same core component (info for example), only the last one is applied.
I was expected all of them to be applied in a chain pattern. They should end up being embedded according to where the Original component is applied.
So, in this example I should see both my titles before and after the info content.

Fix

I pinpointed the issue on system.js where plugins are combined.
As systemExtend() is called with an empty object for dest, we can't retrieve the existing components.

I was able to fix this issue by replacing .reduce(systemExtend, {}) with .reduce(systemExtend, toolbox.getComponents()) on system.js

WDYT ?

[mathis-m]

@tim-lai as already reported for wrapSelectors in #7157 we should think of a general refactor of the system.js where the wrapping is done in appropriate way.

[DayS]

I agree :)

In the meantime, can we expect a small fix on this point and the one in the related issue or is it out of scope for now ?

[tim-lai]

@DayS I added some comments to your PR #7236. Pending some tweaks, this specific issue can get fixed. I do agree that it is safer to add a config option to change the current default behavior.

@mathis-m fyi, I did a quick experiment with both this issue and #7157. The PR associated with this ticket does not address the wrapSelectors referenced in #7157

[tim-lai]

fixed via #7236

[char0n]

Hi everybody,

The expected behavior described in this issue has always been intended. It shouldn't matter if whether the plugins have been registered via presets or passed by the plugins option. Having said that, this is just a bug in the original behavior, and the expected behavior (called chaining in this issue) should be the default one. Wrapping actions and selectors already works like that (ref #7304).

In #7236, the remediation was provided, but unfortunately it also contains hidden bug, where the original components objects is being mutated by the combined plugins because of this line:

 const dest = pluginOptions.pluginLoadType === "chain" ? toolbox.getComponents() : {}

---

Action: we're gonna revert 516e666 and replace it with non-configurable solution. We're going to fix the mutation bug introduced by the line above.

[char0n]

Addressed in #9919