feat: improve types #195
feat: improve types #195
Conversation
|
This branch is running in CodeSandbox. Use the links below to review this PR faster. |
| const valueMap = new Map<keyof Constraints, unknown>(); | ||
| const format = | ||
| schema.formatError ?? | ||
| (defaultFormatError as FormatErrorFunction<Constraints, FormError>); |
There was a problem hiding this comment.
this casting type is currently as lie, I saw that you had a FIXME here, did you have some other idea for going about fixing this?
| @@ -697,7 +698,7 @@ export function parse< | |||
| }); | |||
| } | |||
|
|
|||
| const value = Object.fromEntries(valueMap) as any; | |||
| const value = Object.fromEntries(valueMap) as InferType<Constraints>; | |||
There was a problem hiding this comment.
this is also a little bit of a lie, I currently just forced it to whatever the used variable is expecting there's also a case where it expects both a Partial and T but didn't make sense of the full context, any suggestions here would be helpful.
| @@ -63,15 +63,15 @@ export function getFieldsetConstraint<Source extends yup.AnyObjectSchema>( | |||
| case 'min': | |||
| if ( | |||
| !constraint.min || | |||
| constraint.min < Number(test.params?.min) | |||
| Number(constraint.min) < Number(test.params?.min) | |||
There was a problem hiding this comment.
I casted these, because the original type said they could be a number or string, let me know if you were expecting something else and I'll update accordingly.
| @@ -98,7 +98,7 @@ export function parse<Schema extends yup.AnyObjectSchema>( | |||
| }: { | |||
| name: string; | |||
| intent: string; | |||
| payload: Record<string, any>; | |||
| payload: Record<string, unknown>; | |||
There was a problem hiding this comment.
I changed these all to unknown so that it was clear we weren't handling the payload directly in library code and if there needed coersion it will warn in the future about potential improper data handling
| schema: z.ZodType<T>, | ||
| ): FieldsetConstraint<z.input<Source>> { | ||
| if (schema instanceof z.ZodObject) { | ||
| const result: FieldsetConstraint<z.input<Source>> = {}; | ||
| const result: Record<string, FieldConstraint<unknown>> = {}; |
There was a problem hiding this comment.
the idea here is to cast it to a type that is currently known within this code block
| result[key] = inferConstraint(def); | ||
| } | ||
|
|
||
| return result; | ||
| return result as FieldsetConstraint<z.input<Source>>; |
There was a problem hiding this comment.
and then assume we did our duediligence and hand back the type the caller is expecting.
| } | ||
|
|
||
| if (schema instanceof z.ZodEffects) { | ||
| return resolveFieldsetConstarint(schema.innerType()); | ||
| return resolveFieldsetConstraint(schema.innerType()); |
| @@ -17,7 +17,9 @@ interface PlaygroundProps< | |||
| children: ReactNode; | |||
| } | |||
|
|
|||
| export function Playground({ | |||
| export function Playground< | |||
| TSubmission extends { error: Record<string, string | string[]> | null }, | |||
There was a problem hiding this comment.
I can pull this out if you want, doesn't really matter.
| * @see https://conform.guide/api/react#list | ||
| */ | ||
| export const list = new Proxy<{ | ||
| type List = { |
There was a problem hiding this comment.
the idea here is this is used as our type contract, while the implementation is casted where needed.
| payload: ListIntentPayload<Schema>, | ||
| ): Array<Schema> { | ||
| ): Array<Schema | undefined> { |
There was a problem hiding this comment.
this had to change, there's a possiblity that this doesn't make sense, since I believe there's some optionality happening in the ListIntentPayload type, which might indicate that it might be worth being pulled out or rearanged to make this make sense during actual runtime usage.
any context you have on the operations that take place here would be helpful.
What I did
any