search

swagger-api / swagger-ui

Swagger UI is a collection of HTML, JavaScript, and CSS assets that dynamically generate beautiful documentation from a Swagger-compliant API.
https://swagger.io
Apache License 2.0
26.56k stars 8.96k forks source link

docExpansion=full doesn't seem to work for multiple swagger-ui component instances #6996

Closed thiagoalves closed 1 year ago

thiagoalves commented 3 years ago

Q&A (please complete the following information)

Describe the bug you're encountering

To reproduce...

Steps to reproduce the behavior:

  1. Create a react app and install swagger-ui-react

    npx create-react-app swagger-ui-bug
yarn add swagger-ui-react
yarn start

  2. Edit src/App.js with the following content:

    
import SwaggerUI from "swagger-ui-react"
import "swagger-ui-react/swagger-ui.css"

function App() { return (

); }

export default App;



3. See the result in the browser.  Only one of the components expand the operations

### Expected behavior
Both components should expand the operations

### Screenshots
![image](https://user-images.githubusercontent.com/454835/109051942-d769e200-76b9-11eb-92a8-486a34b69d06.png)

### Additional context or thoughts
The behavior is consistently reproducible.
thiagoalves commented 3 years ago

@mathis-m @hkosova @tim-lai @webron @shockey Sorry for bothering you, but I need to deliver a feature and I am blocked because of this. :\

In my specific case I am creating a plugin that shows ONLY the parameters for a given method / path from an API spec:

const ApiParamsPlugin = (system) => ({
  wrapComponents: {
    operation: (Original, system) => (props) => {
      const op = props.operation.get("op")
      const params = op.get("parameters")   // this is empty when docExpansion != "full" or when I have multiple components in the same app

This works ONLY when I have only one <SwaggerUI> component in my app and docExpansion="full".

Is there any fix / workaround I could apply to unblock this? I will be happy to provide any extra info you might need here.

Thanks

thiagoalves commented 3 years ago

@mathis-m

I have done a lot of experimentation, with YAML and JSON specs referenced side by side in the same React app. A few things I have noticed:

  1. When the specs come from different hosts, it seems to work: 
    
<SwaggerUI url={"https://gist.githubusercontent.com/thiagoalves/2b2c655aa7c812bfb75a08cf837ac104/raw/00c40100a4433c2ad1fa274a13cafcb8a4ad89e0/swagger.yaml"} docExpansion={"full"} />

<SwaggerUI url={"https://petstore.swagger.io/v2/swagger.json"} docExpansion={"full"} />

2. I have created 2 sample files with one operation each. They have different tags assigned to them, but somehow swagger seems to confuse the two:

![image](https://user-images.githubusercontent.com/454835/111503710-c135cb80-8725-11eb-8813-8fe8b02a04f7.png)

The sample files:

openapi: 3.0.0 info: title: ASD version: "123asd" tags:

openapi: 3.0.0 info: title: EFG version: "123efg" tags:

mathis-m commented 3 years ago

@thiagoalves Thanks for all your affort! This issue really bugs me, I also am blocked because of this. Its kind of interesting. I guess this bug is related to DOM. Somehow... Duplicate ids or key I guess that breaks the react DOM or so. But this is just some unqualified guess I have no idea what is going on here.

Looping @tim-lai you probably know more about reacts ecosystem and lifecycle, can you point us in some direction?

mathis-m commented 3 years ago

To sum up in case of having operations collapsed there is no issue. => issue may be in some nested child component of the operation-tag.

When we expand operations simultaneously there will be some unexpected behavior. OperationTags of different swagger-ui instances will be rendered in the wrong instance additionally. IF this happens the wrongly placed component instance will not work. Cant be fixed by collapse and expanding again. => makes sense because they are running in a wrong context. The above stated behavior happens if all paths, tags and operationIds are different:

      <SwaggerUI
        url={
          "https://gist.githubusercontent.com/mathis-m/0a241f7e2a3d82cb5d925a40de9023f9/raw/f000d58f9ebf92cbcd47456949f78466830d60aa/HaLHKNKUHB.txt"
        }
        docExpansion={"full"}
      />

      <SwaggerUI
        url={"https://petstore.swagger.io/v2/swagger.json"}
        docExpansion={"full"}
      />

In case of having same path and tag it will render the operation first in some invalid state (I guess this is because it has again the wrong component of a different instance actually rendered in this place). If you manually collapse this operation and expand it again it will work properly. => IF not expanded simultaneously it will render the correct:

Actually the example stated @thiagoalves does only work some times => I guess some React DOM operations that have different results in case of different timing. Again I think it gets confused about some IDs or Keys but when rendered subsequently there is no issue, in case the timing is some how wrong it will get confused about where to render the component:

<SwaggerUI url={"https://gist.githubusercontent.com/thiagoalves/2b2c655aa7c812bfb75a08cf837ac104/raw/00c40100a4433c2ad1fa274a13cafcb8a4ad89e0/swagger.yaml"} docExpansion={"full"} />

<SwaggerUI url={"https://petstore.swagger.io/v2/swagger.json"} docExpansion={"full"} />
thiagoalves commented 3 years ago

@mathis-m Thanks for the attention

I was able to circumvent the problem by adding a random delay before rendering the component:

  const [load, setLoad] = useState(false);
  useEffect(() => {
    setTimeout(() => setLoad(true), Math.random() * 1000);
  }, []);

  return load && (
      <SwaggerUI
        url={props.apiUrl}
        docExpansion={"full"}
      />
  );

This solves my problem for now. But I am still interested in other ideas you might have in the future.

Best,

mathis-m commented 3 years ago

I think the solution would be to remove all Ids or make them instance specific, eg. prepending some unique nonce per instance.

mathis-m commented 3 years ago

I think the react dom is in some messed up sate because of multiple swagger ui instances. It will randomly apply components to wrong but similar positions.

tim-lai commented 3 years ago

It is possible that there is missing React index keys for child components, to which ReactDOM is unable to accurately identify the intended dom node to attach.

rhwilburn commented 1 year ago

I am also getting this issue in using c# for generation:

app.UseSwaggerUI(c =>
{
       c.DocExpansion(DocExpansion.Full);
 });

where this will not correctly expand the swagger (which is curious as that should have correct context as its within a single document context; but rendered in multiple document context); so I used a swagger UI wrap component as follows, which gives the problem described in this thread (of expanded operations but forever spinning). I can get them to load properly by toggling the operation expander where it will instantly correct itself.

This is the swagger-ui wrap component:

import React from "react";
import OperationDescription from "./operationDescriptionComponent";

export const OperationDescriptionPlugin = function (system: any) {
    return {
        wrapComponents: {
            operation: (Original: any, system: any) => (props: any) => {
                return (
                    <div className="description-wrapper">
                        test
                        <Original {...props} isShown={() => true} />
                    </div>
                );
            },
        },
    };
};

I have 3 <SwaggerUI url="https://localhost:7001/swagger/v1/swagger.json" /> in a single page from the swagger-ui-react library.

Edit: Trying with only a single <SwaggerUI tag fixes the issue using the wrap component which reinforces what others have said around the context being wrong etc between the instances

For the react component at least, there isn't anything against window object that I can see (like there is for non react component).

rhwilburn commented 1 year ago

I have discovered that in my operation wrap component that:

constructor(){
            var method = props.state.operation.get("method");
            var operationPath= props.state.operation.get("path");
            var swagger = this.system.getSystem().spec().toJSON();

            //This line errors as the operationPath is not in the list of paths. This is because the swagger document is completely different to what is in the state object. So this shows the mismatch inside the operation component.
            var description = swagger.json.paths[operationPath]
}

One other thing that may be related potentially is around the use of Redux in the containers:

There is mention here: https://github.com/swagger-api/swagger-ui/issues/4630 of Redux properties being baked into containers. I do wonder if something baked in is causing the properties to not be in sync?

Im new to react so will try to use wrapcomponents where I can to narrow down the errors component wise.

rhwilburn commented 1 year ago

I have been looking into this now from a few different angles, and believe (but haven't yet proven) that its redux conflicting state across the control instances because of the lazy loading nature of operations; and also that usually most of one spec will load fully; and the rest not.

Based on searching for the creation of a redux store, there is only one by the looks: https://github.com/search?q=repo%3Aswagger-api%2Fswagger-ui%20createStore&type=code in https://github.com/swagger-api/swagger-ui/blob/89f04d3f11288b4a63bf1f6dda3a7ca297ed69ff/src/core/system.js#L27 .

If we look at the Redux guide (new to Redux here) https://redux.js.org/usage/isolating-redux-sub-apps (it suggests that subcomponents would have their own stores. In our case its less about subcomponents and more about multiple instances of swagger-ui. Here is a suggested solution using namespacing for this: https://stackoverflow.com/questions/42906358/having-multiple-instance-of-same-reusable-redux-react-components-on-the-same-pag

There is a similar solution in this codebase for namespacing but for examples here: https://github.com/swagger-api/swagger-ui/blob/89f04d3f11288b4a63bf1f6dda3a7ca297ed69ff/src/core/components/examples-select-value-retainer.jsx#L91 so seems like this would be how we should fix this issue assuming its a redux namespacing oriented issue.

The issue that I am seeing (I believe but don't have full proof of it being in this area) is specifically in the non loading of either operation or operationcontainer. (Reminder of this thread here talking about Redux in containers meaning that variables in containers as static copies of properties; such as operationContainer: https://github.com/swagger-api/swagger-ui/issues/4630). That combined with a common redux store (and non namespacing), could be the issue here.

The hierarchy of components & containers are:

BaseLayout (https://github.com/swagger-api/swagger-ui/blob/337fd399935cb8dc467b7dc462917ddaf83db4c7/src/core/components/layouts/base.jsx#L23C20-L23C20) -> Operations (https://github.com/swagger-api/swagger-ui/blob/337fd399935cb8dc467b7dc462917ddaf83db4c7/src/core/components/operations.jsx#L50C53-L50C53) -> OperationsContainer (https://github.com/swagger-api/swagger-ui/blob/337fd399935cb8dc467b7dc462917ddaf83db4c7/src/core/containers/OperationContainer.jsx#L197) -> Operation

I suspect, (again new to Redux so could easily miss something) that its the async loading of operations that shows the redux store being overwritten.

I will keep looking into this. I note that plugins can access the store too; so maybe a plugin solution could be possible until a proper one is possible. State and Store both come from the Redux store and are passed around through this.props between components.

rhwilburn commented 1 year ago

Here is a minimum reproduction of the issue:

https://stackblitz.com/edit/stackblitz-starters-xejdpc?file=src%2FApp.tsx

Note: It doesn't happen every single time but refresh it a few times and you will see the bottom specs etc just spin wait forever. On my non minimal example it happens everytime but I have custom layouts, plugins etc which no doubt extends the possibility of a concurrent conflict where the next spec loads before the first is lazily loaded.

You can use this syntax in browser console:

Array.from(swaggerRefs['sectionc'].system.getStore().getState())

to see the states in each. They seem to be independent in that sectionA state looks like sectionA state still. So perhaps namespacing the elements isn't necessary.

Looking through the code again, this is likely not being called correctly: https://github.com/swagger-api/swagger-ui/blob/89f04d3f11288b4a63bf1f6dda3a7ca297ed69ff/src/core/containers/OperationContainer.jsx#L94C27-L94C27 and perhaps offers some clues (as its what would be called to finish loading the operation).

rhwilburn commented 1 year ago

I have tracked the issue down by forking the code base and reproducing in that code locally.

There are a few steps to setup locally:

1: You must fork the repo 2: Yarn install, yarn build, yarn run dev 3: Yarn link 4: Follow the development instructions around which file and url is being used to serve html add multiple swagger UIs 4: Manually add the source maps using file:/// etc to add them from the dist folder into chrome

Add breakpoints as you wish in vscode etc.

Edited with update:

I have observed the problem in operations.jsx; we are getting from watching this line here: https://github.com/swagger-api/swagger-ui/blob/89f04d3f11288b4a63bf1f6dda3a7ca297ed69ff/src/core/components/operations.jsx#L52

The debugger should show APIs only within a given swagger doc. The image below highlights the problem at this point in the code where webhook and users should not be in the same list of operations (as opertions are per doc).

image

Update: Note that preventing the default tag elements in the first spec did not help fix things (sort of expected but was curious if it was fouling things up. Its not beyond what is seen). It doesn't fix the issue shown in the screenshot; as I can still see multiple services mashed together.

rhwilburn commented 1 year ago

99% sure how the issue is in the resolving of the subtree; a few different angles indicate this. Resolving the subtree populates the specSelector which is always wrong (while the spec itself is always correct).

Specifically issues are NOT in the spec objects that I can see. They are in the UPDATE_RESOLVED_SUBTREE reducer (in reducers.js).

First 3 times it hits that reducer there are 0 items in it (for 3 specs). The next time it has all 3 specs combined together in the selector (replicating what I see in the UI and other debug angles I have taken).

Feels like getting close to the source of this now.

Code to view this is by adding the following log methods to the function in reducers.js:

  [UPDATE_RESOLVED_SUBTREE]: (state, action) => {
    console.log('#################################################')
    console.log('reducers->UPDATE_RESOLVED_SUBTREE');
    console.log(action.payload);
    console.log('#################################################')

    const { value, path } = action.payload
    return state.setIn(["resolvedSubtrees", ...path], fromJSOrdered(value))
  },

Next step to understand why this is behaving strangely (ie why is it called 4 times instead of 3 with 3 specs; ie why is it missing the subtree 3 times; then combining in the 4th attempt).

rhwilburn commented 1 year ago

I believe the issue specifically is the following lines from https://github.com/swagger-api/swagger-ui/blob/21a2ef5b70b10e6ab992502c2231f20337951253/src/core/plugins/spec/actions.js#L267

  requestBatch.push(path)
  requestBatch.system = system

requestBatch gets all of the subtree expansion requests (ie operations population) from this method. They all get pushed into requestBatch from all specs. The code is however putting a single system object back into the requestBatch.

The object isn't in redux (which redux appears to work fine):

https://github.com/swagger-api/swagger-ui/blob/21a2ef5b70b10e6ab992502c2231f20337951253/src/core/plugins/spec/actions.js#L141

I believe the answer is to use Redux to scope it properly. I will give a stab at this locally to see if its correct.

rhwilburn commented 1 year ago

I have a working POC here: https://github.com/swagger-api/swagger-ui/compare/master...rhwilburn:swagger-ui:master

I abandoned a redux based approach:

I had tried to solve via using Redux but its currently scoped around the 'spec' object and that would be a significant redesign I suspect. The issue with the spec object updating is that this.props will change and it gets in an infinite rendering loop with react. Longer term using a redux approach would be better as it would support multiple specs with the same URL (where as I had to make assumptions that spec URL determines uniqueness).

I will tidy this up and do a PR for it.

char0n commented 1 year ago

Fixed by https://github.com/swagger-api/swagger-ui/pull/9050

skyblackhawk commented 2 months ago

Please review this fix, it doesn't work in the latest version of swaggerUI library for property in application.properties SpringBoot3.2 WebFlux Java21: springdoc.swagger-ui.docExpansion=full

<dependency>
    <groupId>org.springdoc</groupId>
    <artifactId>springdoc-openapi-starter-webflux-ui</artifactId>
    <version>2.6.0</version>
</dependency>

Thanks a lot for review and fix it.