rapi-doc / RapiDoc

RapiDoc -WebComponent for OpenAPI Spec
https://rapidocweb.com
MIT License
1.71k stars 285 forks source link

Regression in 7.2.0 - basePath missing from URL in UI #123

Closed peterbroadhurst closed 4 years ago

peterbroadhurst commented 4 years ago

With <script src="https://unpkg.com/rapidoc@7.1.1/dist/rapidoc-min.js"></script> we see the following correct URL in the browser, and the exerciser works correctly:

image

Whereas with latest (or <script src="https://unpkg.com/rapidoc@7.2.0/dist/rapidoc-min.js"></script>) we see the URL loses the basePath and the exerciser is broken:

image

I'm working to isolate the code change, but as this is a potentially regression for other users I wanted to get it raised while I investigate.

peterbroadhurst commented 4 years ago

The regression is caused by this change https://github.com/mrin9/RapiDoc/commit/504af8a5eaaa8f1b995e0522bf8ac155feb1dcf4

Re-instating json-refs in spec-parser.js instead of swagger-client fixes the problem. Digging further to see what's different about the jsonParsedSpec output from the two tools.

peterbroadhurst commented 4 years ago

I believe the bug is a one-liner: https://github.com/mrin9/RapiDoc/blob/8ecfb4d778064b441660e7af51f78078eee5bc66/src/utils/spec-parser.js#L22-L26

See the bottom line overrides the output from convertedSpec.openapi back to specObj.spec in the case of a Swagger 2.0 -> OpenAPI 3.0 conversion.

If I simply put the jsonParsedSpec = specObj.spec before the if block, the problem is resolved.

    jsonParsedSpec = specObj.spec;
    if (specObj.spec.swagger) {
      convertedSpec = await converter.convertObj(specObj.spec, convertOptions);
      jsonParsedSpec = convertedSpec.openapi;
    }
peterbroadhurst commented 4 years ago

I've submitted a PR - fyi @mrin9 in case you're able to review and generate a release for this. I think this will hit anyone using the hosted latest RapiDoc since 7.2.0 was pushed, who have Swagger 2.0 definitions.

mrin9 commented 4 years ago

thanks a lot for spotting it early and the quick PR. did a bug-fix release 7.2.1 and added a test case to avoid this in future