Closed glowcloud closed 1 month ago
The root issue is that we're mutating parent and subsequent visitors have no idea about this change. This is a regression introduced by integrating ApiDOM Dereference Architecture 2.0.
We have couple of options how to resolve this:
significant
tree modifications and will dispatch subsequent visitors in separate passes. We don't have this mechanism currently because we explicitly avoided it. It makes performance unpredictable. But I can imagine if you don't care about performance (CLI usage for example), you just turn it on with option and don't care how your visitors really look like.all-of
, parameters
and properties
visitors.Decision: we will go for option 4. It requires the least amount of effort to fix the issue, it requires minimal repeated traversal of the same nodes and is aligned of how ApiDOM traversal was original design to be performed.
I've researched standardized approach to the problem we currently have - it's called requeueing the traversal
and I'm currently in the process to implement it in ApiDOM. There are two approaches to the requeueing:
Babel is using automatic requeueing as it has an abstraction facilitated via NodePath
.
Our traversal mechanism is more generic and not bound to particular structure (it handles parser CSTs, ASTs and ApiDOM). It was originally build as immutable, but for dereferencing we needed to utilize mutations. Every time the mutation is introduced that significantly modifies the ApiDOM tree (e.g. replacing current node), we need to manually dispatch requeueing traversal
.
For solving this issue we technically didn't need complex requeuing mechanism. ApiDOM traversal natively supported immutable current node replacement. In https://github.com/swagger-api/apidom/issues/4120 we added support for mutable current node replacement - this is all that we currently need.
Upstream ApiDOM issue https://github.com/swagger-api/apidom/issues/4120
Content & configuration
Swagger/OpenAPI definition:
Describe the bug you're encountering
The
allOf
properties are not merged into the parent schema if the schema was referenced. The dereferenced schema in the response will still have theallOf
keyword:while the schema itself will be merged:
The issue here is that the parent elements get mutated by the
DereferenceVisitor
: https://github.com/swagger-api/swagger-js/blob/fa77bfd689e070c2e1fe7f6f58a7cd5b9368ebea/src/resolver/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/visitors/dereference.js#L776-L780 and the subsequent visitors do not see this change.We get the correct result if instead of merging the
AllOfVisitor
with theDereferenceVisitor
and visiting here: https://github.com/swagger-api/swagger-js/blob/fa77bfd689e070c2e1fe7f6f58a7cd5b9368ebea/src/resolver/apidom/reference/dereference/strategies/openapi-3-1-swagger-client/index.js#L107-L118we did a second merge and visit with only the
AllOfVisitor
on the dereferenced element:because we get updated elements.
Expected behavior
The updated elements need to be passed to the visitors after dereferencing, as this issue will affect all of them.