davidmdm / myzod

MIT License
315 stars 20 forks source link

Add option of coerce for EnumType #22

Closed kaedub closed 4 years ago

kaedub commented 4 years ago

I have had to do some hacky workarounds to handle string case sensitivity when parsing an EnumType so I created this and was hoping to get it adopted. If this feature could cause problems in other places or if there is something I should change please let me know. Thanks!

davidmdm commented 4 years ago

@kaedub Hi! Sorry for not getting back to you quicker!

I see what you are trying to do and I think it's a fine change. If you can just add a little something in the readme documenting this addition to the EnumType API, I will merge it and release a new version on npm!

Thanks for your efforts again!

kaedub commented 4 years ago

@davidmdm Thanks for the feedback. I went ahead and added to the Enum section of the readme. I noticed that I had not added the same logic to the Enum's check method so I added that in here too. I assume if we tell it to coerce strings to ignore casing, it should do so for both parse and check.

davidmdm commented 4 years ago

No I think we should remove the logic from the check. I omitted a check method for most types because I wanted type coercion as a base feature of myzod, and I can't seem to make them play nice together.

If we leave the logic in you break the type system:

enum RGB { red, green, blue }
const RGBSchema = myzod.enum(RGB).coerce('lower');

const maybeGreen = 'GREEN';
const g = RGBSchema.parse(maybeGreen);
// here g is of type RGB which is true because parse will have returned 'green'. 

if (RGBSchema.check(maybeGreen)) {
  // maybeGreen is typed as RGB here but the underlying value isn't and we've broken type-safety.
}

So I would remove it from the check, and make a note in the readme that coercion only affects the parse and try apis but not check.

Sounds good?

kaedub commented 4 years ago

@davidmdm Yeah sounds good.