Fix #7052 circular reference

[kyle-apex]

Fixes #7052 by removing processing of circular references

Description

Updated fromJSOrdered to handle circular references using the same implementation as "fromJS" in the "immutable-js" library: https://github.com/immutable-js/immutable-js/blob/master/src/fromJS.js

Rather than throw an error as "fromJS" does, this fix represents the circular reference with a Map or List depending on whether it's an array or object.

Motivation and Context

Fixes #7052

How Has This Been Tested?

Added new unit tests and manually tested against the definition provided #7052 as well as my own circular reference case.

Screenshots (if appropriate):

Checklist

My PR contains...

• [ ] No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• [ ] Dependency changes (any modification to dependencies in package.json)
• [x] Bug fixes (non-breaking change which fixes an issue)
• [ ] Improvements (misc. changes to existing features)
• [ ] Features (non-breaking change which adds functionality)

My changes...

• [ ] are breaking changes to a public API (config options, System API, major UI change, etc).
• [ ] are breaking changes to a private API (Redux, component props, utility functions, etc.).
• [ ] are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• [x] are not breaking changes.

Documentation

• [x] My changes do not require a change to the project documentation.
• [ ] My changes require a change to the project documentation.
• [ ] If yes to above: I have updated the documentation accordingly.

Automated tests

• [ ] My changes can not or do not need to be tested.
• [x] My changes can and should be tested by unit and/or integration tests.
• [x] If yes to above: I have added tests to cover my changes.
• [ ] If yes to above: I have taken care to cover edge cases in my tests.
• [x] All new and existing tests passed.

[tim-lai]

@char0n Can you take a look at this? Imo, this PR would mask over the cases where a circular reference still exists, but is this aligned with how swagger-client expects swagger-ui to handle circular reference errors?

[kyle-apex]

Is there anything I can do to help move this along?

Circular type references are valid in schema definitions.

However, the current behavior when expanding a section with such a schema is that the page appears to freeze (section not loaded) because there is a max call stack exceeded issue with fromJSOrdered.

This change shouldn't impact any behavior other than avoiding max call stack exceeded errors from valid schemas that have circular type references.

[char0n]

Can you take a look at this? Imo, this PR would mask over the cases where a circular reference still exists, but is this aligned with how swagger-client expects swagger-ui to handle circular reference errors?

@tim-lai as @kyle-apex mentioned, circular references (or in this case cyclic objects) can naturally occur in dereferenced OAS definition which is the product of resolution when using Schema Objects.

@kyle-apex thanks for you work! Looking in this PR now.

[char0n]

@kyle-apex can you please describe exact Steps Of Reproduction on fixture from #7052? I tried to reproduced on https://editor.swagger.io/ but haven't seen Maximum call stack size exceeded.

[kyle-apex]

@char0n after far too long of an exploration (because I wasn't reproducing it using the url), I discovered that the ciruclar reference issue only occurs when the .json is loaded via a relative path like so:

const ui = SwaggerUIBundle({ url: "./openapi.json",

That means it's probably a fix needed with regard to the differences in how the exact same swagger.json is loaded via relative path versus url.

This works fine:

const ui = SwaggerUIBundle({ url: "https://raw.githubusercontent.com/x-du/swagger-ui/CircularReferBug/dev-helpers/openapi.json",

[char0n]

@kyle-apex thanks, I confirm I could reproduce. So the first thing we need to figure out what is difference between loading mechanism and where. And then fix that particular buggy place, would you agree?

[yggdrasil-tynor]

I can confirm this is still an issue when loading swagger specs via a relative path