From 284703d3c275448d87fa4f599cbee390adf76e17 Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Sun, 2 May 2021 09:59:59 +0200 Subject: [PATCH 1/8] fix: wrapComponents on plugins was not being chained when targeting core components, This commit provides a backward compatible mechanism --- src/core/index.js | 6 ++++++ src/core/system.js | 13 ++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/core/index.js b/src/core/index.js index 000b1d3c8d0..b70089e64a6 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -93,6 +93,11 @@ export default function SwaggerUI(opts) { plugins: [ ], + // Behavior during plugin registration. Can be : + // - null (default) : the current behavior for backward compatibility + // - chain : chain wrapComponents when targeting the same core component + pluginsBehavior: null, + // Initial state initialState: { }, @@ -118,6 +123,7 @@ export default function SwaggerUI(opts) { configs: constructorConfig.configs }, plugins: constructorConfig.presets, + pluginsBehavior: constructorConfig.pluginsBehavior, state: deepExtend({ layout: { layout: constructorConfig.layout, diff --git a/src/core/system.js b/src/core/system.js index 30a41ddf400..10125635f16 100644 --- a/src/core/system.js +++ b/src/core/system.js @@ -35,6 +35,7 @@ export default class Store { deepExtend(this, { state: {}, plugins: [], + pluginsBehavior: null, system: { configs: {}, fn: {}, @@ -63,7 +64,7 @@ export default class Store { } register(plugins, rebuild=true) { - var pluginSystem = combinePlugins(plugins, this.getSystem()) + var pluginSystem = combinePlugins(plugins, this.getSystem(), this.pluginsBehavior) systemExtend(this.system, pluginSystem) if(rebuild) { this.buildSystem() @@ -310,19 +311,21 @@ export default class Store { } -function combinePlugins(plugins, toolbox) { +function combinePlugins(plugins, toolbox, behavior) { if(isObject(plugins) && !isArray(plugins)) { return assignDeep({}, plugins) } if(isFunc(plugins)) { - return combinePlugins(plugins(toolbox), toolbox) + return combinePlugins(plugins(toolbox), toolbox, behavior) } if(isArray(plugins)) { + const dest = behavior === 'chain' ? toolbox.getComponents() : {} + return plugins - .map(plugin => combinePlugins(plugin, toolbox)) - .reduce(systemExtend, {}) + .map(plugin => combinePlugins(plugin, toolbox, behavior)) + .reduce(systemExtend, dest) } return {} From b9938b2e54d4d6e66318eff7904b4f8c4cb45614 Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Sun, 2 May 2021 14:42:48 +0200 Subject: [PATCH 2/8] chore: Add unit test for wrapComponent wrapping --- test/unit/core/system/wrapComponent.jsx | 76 +++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/test/unit/core/system/wrapComponent.jsx b/test/unit/core/system/wrapComponent.jsx index 0605b0d510a..826e5fdbb62 100644 --- a/test/unit/core/system/wrapComponent.jsx +++ b/test/unit/core/system/wrapComponent.jsx @@ -5,7 +5,7 @@ import System from "core/system" describe("wrapComponents", () => { describe("should wrap a component and provide a reference to the original", () => { - it("with stateless components", function(){ + it("with stateless components", function () { // Given const system = new System({ plugins: [ @@ -40,7 +40,7 @@ describe("wrapComponents", () => { expect(children.eq(1).text()).toEqual("Wrapped component") }) - it("with React classes", function(){ + it("with React classes", function () { class MyComponent extends React.Component { render() { return
{this.props.name} component
@@ -86,7 +86,7 @@ describe("wrapComponents", () => { }) }) - it("should provide a reference to the system to the wrapper", function(){ + it("should provide a reference to the system to the wrapper", function () { // Given @@ -137,7 +137,7 @@ describe("wrapComponents", () => { expect(children.eq(1).text()).toEqual("WOW much data") }) - it("should wrap correctly when registering more plugins", function(){ + it("should wrap correctly when registering more plugins", function () { // Given @@ -163,7 +163,7 @@ describe("wrapComponents", () => { }) mySystem.register([ - function() { + function () { return { // Wrap the component and use the system wrapComponents: { @@ -191,7 +191,71 @@ describe("wrapComponents", () => { expect(children.eq(1).text()).toEqual("WOW much data") }) - it("should wrap correctly when building a system twice", function(){ + it("should wrap correctly when registering multiple plugins targeting the same component", function () { + + // Given + + const mySystem = new System({ + pluginsBehavior: 'chain', + plugins: [ + () => { + return { + components: { + wow: () =>
Original component
+ } + } + } + ] + }) + + mySystem.register([ + () => { + return { + wrapComponents: { + wow: (OriginalComponent, system) => (props) => { + return + +
Injected after
+ + } + } + } + }, + () => { + return { + wrapComponents: { + wow: (OriginalComponent, system) => (props) => { + return +
Injected before
+ + + } + } + } + } + ]) + + // Then + let Component = mySystem.getSystem().getComponents("wow") + const wrapper = render() + + const container2 = wrapper.children().first() + expect(container2[0].name).toEqual("container2") + + const children2 = container2.children() + expect(children2.length).toEqual(2) + expect(children2[0].name).toEqual("div") + expect(children2.eq(0).text()).toEqual("Injected before") + expect(children2[1].name).toEqual("container1") + + const children1 = children2.children() + expect(children1.length).toEqual(2) + expect(children1.eq(0).text()).toEqual("Original component") + expect(children1[0].name).toEqual("div") + expect(children1.eq(1).text()).toEqual("Injected after") + }) + + it("should wrap correctly when building a system twice", function () { // Given From c2dd34714ca5725a4530a07adfed0aa6468601db Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Mon, 10 May 2021 22:10:12 +0200 Subject: [PATCH 3/8] Fix lint errors --- src/core/system.js | 2 +- test/unit/core/system/wrapComponent.jsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/system.js b/src/core/system.js index 10125635f16..afaf2631a88 100644 --- a/src/core/system.js +++ b/src/core/system.js @@ -321,7 +321,7 @@ function combinePlugins(plugins, toolbox, behavior) { } if(isArray(plugins)) { - const dest = behavior === 'chain' ? toolbox.getComponents() : {} + const dest = behavior === "chain" ? toolbox.getComponents() : {} return plugins .map(plugin => combinePlugins(plugin, toolbox, behavior)) diff --git a/test/unit/core/system/wrapComponent.jsx b/test/unit/core/system/wrapComponent.jsx index 826e5fdbb62..bb5ac34cd45 100644 --- a/test/unit/core/system/wrapComponent.jsx +++ b/test/unit/core/system/wrapComponent.jsx @@ -196,7 +196,7 @@ describe("wrapComponents", () => { // Given const mySystem = new System({ - pluginsBehavior: 'chain', + pluginsBehavior: "chain", plugins: [ () => { return { From ba57cf8c34139d8dd31e6b9bbd879c1d8f893bac Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Wed, 19 May 2021 12:14:11 +0200 Subject: [PATCH 4/8] chore: Replace pluginsBehavior by a pluginsOptions object with loadType parameter --- src/core/index.js | 12 +++++++----- src/core/system.js | 12 ++++++------ test/unit/core/system/wrapComponent.jsx | 4 +++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/core/index.js b/src/core/index.js index b70089e64a6..e5f68527ad4 100644 --- a/src/core/index.js +++ b/src/core/index.js @@ -93,10 +93,12 @@ export default function SwaggerUI(opts) { plugins: [ ], - // Behavior during plugin registration. Can be : - // - null (default) : the current behavior for backward compatibility - // - chain : chain wrapComponents when targeting the same core component - pluginsBehavior: null, + pluginsOptions: { + // Behavior during plugin registration. Can be : + // - legacy (default) : the current behavior for backward compatibility – last plugin takes precedence over the others + // - chain : chain wrapComponents when targeting the same core component + pluginLoadType: "legacy" + }, // Initial state initialState: { }, @@ -123,7 +125,7 @@ export default function SwaggerUI(opts) { configs: constructorConfig.configs }, plugins: constructorConfig.presets, - pluginsBehavior: constructorConfig.pluginsBehavior, + pluginsOptions: constructorConfig.pluginsOptions, state: deepExtend({ layout: { layout: constructorConfig.layout, diff --git a/src/core/system.js b/src/core/system.js index afaf2631a88..fd14b4807f6 100644 --- a/src/core/system.js +++ b/src/core/system.js @@ -35,7 +35,7 @@ export default class Store { deepExtend(this, { state: {}, plugins: [], - pluginsBehavior: null, + pluginsOptions: {}, system: { configs: {}, fn: {}, @@ -64,7 +64,7 @@ export default class Store { } register(plugins, rebuild=true) { - var pluginSystem = combinePlugins(plugins, this.getSystem(), this.pluginsBehavior) + var pluginSystem = combinePlugins(plugins, this.getSystem(), this.pluginsOptions.pluginLoadType) systemExtend(this.system, pluginSystem) if(rebuild) { this.buildSystem() @@ -311,20 +311,20 @@ export default class Store { } -function combinePlugins(plugins, toolbox, behavior) { +function combinePlugins(plugins, toolbox, pluginLoadType) { if(isObject(plugins) && !isArray(plugins)) { return assignDeep({}, plugins) } if(isFunc(plugins)) { - return combinePlugins(plugins(toolbox), toolbox, behavior) + return combinePlugins(plugins(toolbox), toolbox, pluginLoadType) } if(isArray(plugins)) { - const dest = behavior === "chain" ? toolbox.getComponents() : {} + const dest = pluginLoadType === "chain" ? toolbox.getComponents() : {} return plugins - .map(plugin => combinePlugins(plugin, toolbox, behavior)) + .map(plugin => combinePlugins(plugin, toolbox, pluginLoadType)) .reduce(systemExtend, dest) } diff --git a/test/unit/core/system/wrapComponent.jsx b/test/unit/core/system/wrapComponent.jsx index bb5ac34cd45..594167cec4c 100644 --- a/test/unit/core/system/wrapComponent.jsx +++ b/test/unit/core/system/wrapComponent.jsx @@ -196,7 +196,9 @@ describe("wrapComponents", () => { // Given const mySystem = new System({ - pluginsBehavior: "chain", + pluginsOptions: { + pluginLoadType: "chain" + }, plugins: [ () => { return { From 92fc82a24af6140cdaadb7979d560cf914d2a856 Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Wed, 19 May 2021 21:52:50 +0200 Subject: [PATCH 5/8] doc: Add documentation about the new pluginsOptions configuration --- docs/usage/configuration.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 39ee5cd283f..17faa008e64 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -39,9 +39,16 @@ Read more about the plugin system in the [Customization documentation](/docs/cus Parameter name | Docker variable | Description --- | --- | ----- `layout` | _Unavailable_ | `String="BaseLayout"`. The name of a component available via the plugin system to use as the top-level layout for Swagger UI. +`pluginsOptions` | _Unavailable_ | `Object`. A Javascript object to configure plugin integration and behaviors (see below). `plugins` | _Unavailable_ | `Array=[]`. An array of plugin functions to use in Swagger UI. `presets` | _Unavailable_ | `Array=[SwaggerUI.presets.ApisPreset]`. An array of presets to use in Swagger UI. Usually, you'll want to include `ApisPreset` if you use this option. +##### Plugins options + +Parameter name | Docker variable | Description +--- | --- | ----- +`pluginLoadType` | _Unavailable_ | `String=["legacy", "chain"]`. Control behavior of plugins when targeting the same component with wrapComponent.
- `legacy` (default) : last plugin takes precedence over the others
- `chain` : chain wrapComponents when targeting the same core component, allowing multiple plugins to wrap the same component + ##### Display Parameter name | Docker variable | Description From cdd7fcc500b00b51eaa363650bc8cd5233f04403 Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Wed, 19 May 2021 22:00:00 +0200 Subject: [PATCH 6/8] doc: Add a sidenote on plugin-api page --- docs/customization/plugin-api.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/customization/plugin-api.md b/docs/customization/plugin-api.md index defba7228ce..e5d1846c59d 100644 --- a/docs/customization/plugin-api.md +++ b/docs/customization/plugin-api.md @@ -388,6 +388,10 @@ const MyWrapComponentPlugin = function(system) { } ``` +**Note:** + +If you have multiple plugins wrapping the same component, you may want to change the [`pluginsOptions.pluginLoadType`](/docs/usage/configuration.md#Plugins-options) parameter to `chain`. + #### `rootInjects` The `rootInjects` interface allows you to inject values at the top level of the system. From 0fe6fbf3f5540bdea6ca6d2a9e79c693b2f70c00 Mon Sep 17 00:00:00 2001 From: Tim Lai Date: Wed, 19 May 2021 13:39:26 -0700 Subject: [PATCH 7/8] Apply suggestions from code review --- src/core/system.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/system.js b/src/core/system.js index fd14b4807f6..69e6e0ba307 100644 --- a/src/core/system.js +++ b/src/core/system.js @@ -64,7 +64,7 @@ export default class Store { } register(plugins, rebuild=true) { - var pluginSystem = combinePlugins(plugins, this.getSystem(), this.pluginsOptions.pluginLoadType) + var pluginSystem = combinePlugins(plugins, this.getSystem()) systemExtend(this.system, pluginSystem) if(rebuild) { this.buildSystem() From 6c0cf285d48bdfdecebc641a839ef7456dff8986 Mon Sep 17 00:00:00 2001 From: Damien Villeneuve Date: Thu, 20 May 2021 10:39:31 +0200 Subject: [PATCH 8/8] refacto: Apply code review suggestions about combinePlugin call --- src/core/system.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/system.js b/src/core/system.js index 69e6e0ba307..a756039c975 100644 --- a/src/core/system.js +++ b/src/core/system.js @@ -64,7 +64,7 @@ export default class Store { } register(plugins, rebuild=true) { - var pluginSystem = combinePlugins(plugins, this.getSystem()) + var pluginSystem = combinePlugins(plugins, this.getSystem(), this.pluginsOptions) systemExtend(this.system, pluginSystem) if(rebuild) { this.buildSystem() @@ -311,20 +311,20 @@ export default class Store { } -function combinePlugins(plugins, toolbox, pluginLoadType) { +function combinePlugins(plugins, toolbox, pluginOptions) { if(isObject(plugins) && !isArray(plugins)) { return assignDeep({}, plugins) } if(isFunc(plugins)) { - return combinePlugins(plugins(toolbox), toolbox, pluginLoadType) + return combinePlugins(plugins(toolbox), toolbox, pluginOptions) } if(isArray(plugins)) { - const dest = pluginLoadType === "chain" ? toolbox.getComponents() : {} + const dest = pluginOptions.pluginLoadType === "chain" ? toolbox.getComponents() : {} return plugins - .map(plugin => combinePlugins(plugin, toolbox, pluginLoadType)) + .map(plugin => combinePlugins(plugin, toolbox, pluginOptions)) .reduce(systemExtend, dest) }