Skip to content

fix(resolver): avoid duplicating enum values when nested in allOf #3939

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jun 3, 2025

Conversation

glowcloud
Copy link
Contributor

No description provided.

@char0n
Copy link
Member

char0n commented May 30, 2025

This change might have performance implications as we're stepping out of ApiDOM realm into JavaScript realm on possibly big chunks of data.

return (targetElement, sourceElement) => {
if (isArrayElement(targetElement) && isArrayElement(sourceElement)) {
const primitiveElements = new ArrayElement([
...targetElement.findElements(isPrimitiveElement),
Copy link
Member

@char0n char0n Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPrimitiveElement is more about distinction between generic APiDOM elements and Semantic ones. info (from OpenAPI spec) vs object. So this doesn't really do what is was intended to do.

Let's try to put all element inside one array, and R.uniqWith with custom comparator which always returns false to array/object and bool for antyhing elase (use .equals for ApiDOM realm comparisons).

customMerge: (keyElement) => {
if (toValue(keyElement) === 'enum') {
return (targetElement, sourceElement) => {
if (isArrayElement(targetElement) && isArrayElement(sourceElement)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (isArrayElement(targetElement) && isArrayElement(sourceElement)) {
if (includesClasses(['json-schema-enum'], targetElement) && includesClasses(['json-schema-enum'], sourceElement)) {

return a.equals(toValue(b));
};
const mergedElements = targetElement.concat(sourceElement);
const uniqueElements = uniqWith(areElementsEqual)(mergedElements.content);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const uniqueElements = uniqWith(areElementsEqual)(mergedElements.content);
const clone = cloneShallow(targetElement)
clone.content = uniqWith(areElementsEqual)([...targetElement.content, ...sourceElement.content]);
return clone;

Copy link
Member

@char0n char0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment.

@glowcloud glowcloud merged commit 6caddd7 into master Jun 3, 2025
8 checks passed
@glowcloud glowcloud deleted the fix-enum-merge branch June 3, 2025 08:46
swagger-bot pushed a commit that referenced this pull request Jun 3, 2025
## [3.35.4](v3.35.3...v3.35.4) (2025-06-03)

### Bug Fixes

* **resolver:** avoid duplicating enum values when nested in allOf ([#3939](#3939)) ([6caddd7](6caddd7))
@swagger-bot
Copy link
Contributor

🎉 This PR is included in version 3.35.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants