Skip to content

Conversation

@tsmith023
Copy link
Collaborator

@tsmith023 tsmith023 commented Nov 21, 2024

This PR adjusts the generic of the nestedProperties field in NestedDataTypeConfig when dataType has the type ObjectDataType so that there are no compiler errors when strict: false when defining the nestedProperties field of a property type with object or object[] data type

In short, by making use of NestedPropertyConfigCreate, the same issue of strict: false not resolving the conditional type T extends undefined ? ... : ... is encountered. With this change, we avoid that and so remove the bug

Copy link

@orca-security-eu orca-security-eu bot left a comment

Choose a reason for hiding this comment

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

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

dataType: ObjectDataType;
/** only for object types */
nestedProperties?: NestedPropertyConfigCreate<T, ObjectDataType>[];
nestedProperties?: NestedPropertyCreate<T>[];
Copy link
Contributor

Choose a reason for hiding this comment

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

This fix may work @tsmith023
Just confirming in a short period

@weaviate-git-bot
Copy link

Great to see you again! Thanks for the contribution.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

Copy link
Contributor

@pacyL2K19 pacyL2K19 left a comment

Choose a reason for hiding this comment

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

I just tested this locally and it looks fine
Approving this

@tsmith023 tsmith023 merged commit 465407d into main Nov 22, 2024
11 checks passed
@tsmith023 tsmith023 deleted the fix/pr-#231 branch November 22, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants