Skip to content
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

Validate array uniqueness #2672

Merged
merged 15 commits into from
Apr 23, 2024

Conversation

joaoGabriel55
Copy link

@joaoGabriel55 joaoGabriel55 commented Aug 21, 2023

From discussion: #2316

TODO

  • Implement array of primitives uniqueness
  • Implement array of complex objects uniqueness

Thanks for the help: @ramos-ph and @fwoelffel

@netlify
Copy link

netlify bot commented Aug 21, 2023

Deploy Preview for guileless-rolypoly-866f8a ready!

Name Link
🔨 Latest commit 68d83b7
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/66283b3cf8e5c5000892af19
😎 Deploy Preview https://deploy-preview-2672--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@joaoGabriel55
Copy link
Author

What suggestions do you have for array of complex objects uniqueness? I have one that would be make the objects elements from array flatten to simplifies the comparison between.

@aarontravass
Copy link

What suggestions do you have for array of complex objects uniqueness? I have one that would be make the objects elements from array flatten to simplifies the comparison between.

Class-validator seems to be doing a simple comparison

https://github.com/typestack/class-validator/blob/262a3c6369b9a9fc81846a39b62b6a2a21bd8fab/src/decorator/array/ArrayUnique.ts#L7-L20

@joaoGabriel55
Copy link
Author

@colinhacks could you take a look, when you have a free time?

@aarontravass
Copy link

@joaoGabriel55 As discussed here, #2316 (reply in thread), would it be possible to add an options which would display duplicate items?

@joaoGabriel55
Copy link
Author

@joaoGabriel55 As discussed here, #2316 (reply in thread), would it be possible to add an options which would display duplicate items?

yeah!

src/types.ts Outdated Show resolved Hide resolved
@fwoelffel
Copy link

@colinhacks any thoughts about this?

@@ -1975,6 +1975,11 @@ export interface ZodArrayDef<T extends ZodTypeAny = ZodTypeAny>
exactLength: { value: number; message?: string } | null;
minLength: { value: number; message?: string } | null;
maxLength: { value: number; message?: string } | null;
uniqueness: {
identifier?: ArrayUniqueIdentifier;
message?: string | ((duplicateElements: any) => string);

Choose a reason for hiding this comment

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

Suggested change
message?: string | ((duplicateElements: any) => string);
message?: string | ((duplicateElements: T[]) => string);

Copy link
Author

Choose a reason for hiding this comment

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

I tried this, but some types of errors like this:

Type '{ [k in keyof T]: ZodOptional<T[k]>; }' does not satisfy the constraint 'ZodRawShape'.
  Type 'ZodOptional<T[k]>' is not assignable to type 'ZodTypeAny'.
    The types returned by 'refine(...).refine(...).optional()' are incompatible between these types.
      Type 'ZodOptional<ZodEffects<ZodEffects<ZodOptional<T[k]>, any, T[k]["_input"] | undefined>, any, T[k]["_input"] | undefined>>' is not assignable to type 'ZodOptional<ZodEffects<ZodEffects<ZodType<any, any, any>, any, any>, any, any>>'.
        Type 'ZodEffects<ZodType<any, any, any>, any, any>' is not assignable to type 'ZodEffects<ZodOptional<T[k]>, any, T[k]["_input"] | undefined>'.
Type 'Options[number]' does not satisfy the constraint 'ZodType<any, any, any>'.ts(2344)

Choose a reason for hiding this comment

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

I'll try to have a look at this during the weekend ;)

? params.message
: errorUtil.toString(params.message);
: errorUtil.toString(params?.message);

Choose a reason for hiding this comment

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

Suggested change
: errorUtil.toString(params?.message);
: errorUtil.toString(params.message);

Copy link
Author

Choose a reason for hiding this comment

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

I need to put this ? because the params can be not passed (undefined) on the .unique method.

Choose a reason for hiding this comment

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

My bad

@colinhacks
Copy link
Owner

colinhacks commented Apr 23, 2024

Thanks! I like this a lot, especially identifier. I'm merging this into the v4 branch. Some version of this will land in Zod 4, but — as a heads up — it may be a while, and this feature/API will likely undergo some changes in the interim.

@colinhacks colinhacks force-pushed the validate-array-uniqueness branch from 148dfbd to 68d83b7 Compare April 23, 2024 22:50
@colinhacks colinhacks changed the base branch from master to v4 April 23, 2024 22:50
@colinhacks colinhacks merged commit 0e4583d into colinhacks:v4 Apr 23, 2024
20 checks passed
colinhacks added a commit to ytsunekawa/zod that referenced this pull request May 3, 2024
* feat: implement primitive array uniqueness

* chore: update yarn.lock

* feat: implement array of complex objects uniqueness

* chore: rollback yarn.lock

* refactor: minor update _arrayUnique

* chore: minor change yarn.lock

* feat: implement custom message support

* feat: add showDuplicates param

* chore: TS types update

* chore: TS types II

* refactor: ArrayMessageFunction

* refactor: simplify array uniqueness implementation

* docs: add documentation for array uniqueness

* Regen lock

* Fix readme

---------

Co-authored-by: Frederic Woelffel <[email protected]>
Co-authored-by: Colin McDonnell <[email protected]>
@MentalGear
Copy link

Cool, but I do not get why the most idiomatic approach, z.array(z.string()).unique(), is not supported ?
@joaoGabriel55

@fwoelffel
Copy link

Cool, but I do not get why the most idiomatic approach, z.array(z.string()).unique(), is not supported ? @joaoGabriel55

Maybe I'm mistaken, but z.array(z.string()).unique() is supported 🤔

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.

5 participants