remix-params-helper icon indicating copy to clipboard operation
remix-params-helper copied to clipboard

Support ZodIntersection in `useFormInputProps`, ZodPipeline in `getInputProps`

Open solomonhawk opened this issue 1 year ago • 5 comments

Fixes #34 Fixes #35

NOTE: This bumps the peer dep on zod to 3.20.0 for ZodIntersection support.

  • Adds a step to useFormInputProps that handles the case where the passed-in ZodType is something other than a ZodObject (e.g. ZodIntersection/ZodEffects)
  • Adds clauses to getInputProps to handle both ZodEffects (recurse into inner schema) and ZodPipeline (infer input type from the pipeline's out type)
  • Removes generics from calls to getParams in example app - these were yielding type errors and didn't seem to be necessary (type is inferred from schema)
  • (maybe) Fixes a bug with ZodOptional - see https://github.com/kiliman/remix-params-helper/commit/81218795db419ab2e08d5923d773f2bbee80d25a

@kiliman I'm not 100% confident in these changes so a close eye would be appreciated. Per my use case and the tests it seems to address the need. I would welcome additional test cases to consider.

I don't have a good example use case for when a field is itself a ZodIntersection e.g.:

z.object({
  field: z.intersection(z.object({ a: z.string() }), z.object({ b: z.number() }))
})

I cleaned up a few unused variables in the process of making these changes.

I thought about support for z.union([...schemas]) and found it difficult to determine what the correct behavior should be, given that unions model OR types, which could lead to confusing situations where an input's props would need to be e.g. text or number.


In case anyone is curious, I found intersections to be crucial when modeling schemas with refinements on the base type (.superRefine calls that need multiple fields in order to layer on additional cross-field validation constraints). Without them, a failed parse on any of the schema fields prevents the refinement from even running which means you only get some of the errors.

More detailed code sample

Not great, dates constraint violation isn't caught when base schema isn't successfully parsed

const schema = z.object({
  name: z.string(), // required string
  startAt: z.date(),
  endAt: z.date(),
}).superRefine(({ startAt, endAt }, ctx) => {
  if (startAt >= endAt) {
    ctx.addIssue({
      code: z.ZodIssueCode.custom,
      message: 'Start date must be before end date',
    })
  }
})

// schema with invalid dates AND missing name
schema.parse({
  name: undefined,
  startAt: new Date('2021-01-01'),
  endAt: new Date('2020-01-01')
})

// 😔 errors do not include date validation from refinement
// ZodError: [
//   {
//     "code": "invalid_type",
//     "expected": "string",
//     "received": "undefined",
//     "path": [
//       "name"
//     ],
//     "message": "Required"
//   }
// ]

// ----------------------

// schema with valid name and invalid dates, refinement validation runs as expected
schema.parse({
  name: "Barry Bluejeans",
  startAt: new Date('2021-01-01'),
  endAt: new Date('2020-01-01')
})

// ZodError: [
//   {
//     "code": "custom",
//     "message": "Start date must be before end date",
//     "path": []
//   }
// ]

Better, intersection allows refinements to produce errors even when other portions of the schema are invalid

const baseSchema = z.object({
  name: z.string(), // required string
})

const datesSchema = z.object({
  startAt: z.date(),
  endAt: z.date(),
}).superRefine(({ startAt, endAt }, ctx) => {
  if (startAt >= endAt) {
    ctx.addIssue({
      code: z.ZodIssueCode.custom,
      message: 'Start date must be before end date',
    })
  }
})

const schema = z.intersection(baseSchema, datesSchema)

schema.parse({
  startAt: new Date('2021-01-01'),
  endAt: new Date('2020-01-01')
})

// 🎉 both errors are reported, refinement ran even though the baseSchema was invalid
// ZodError: [
//   {
//     "code": "invalid_type",
//     "expected": "string",
//     "received": "undefined",
//     "path": [
//       "name"
//     ],
//     "message": "Required"
//   },
//   {
//     "code": "custom",
//     "message": "Start date must be before end date",
//     "path": []
//   }
// ]

solomonhawk avatar May 02 '23 21:05 solomonhawk

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore [email protected] bar@* or ignore all packages with @SocketSecurity ignore-all

⚠️ Please accept the latest app permissions to ensure bot commands work properly. Accept the new permissions here.

Pull request alert summary
Issue Status
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues

📊 Modified Dependency Overview:

⬆️ Updated Package Version Diff Added Capability Access +/- Transitive Count Publisher
[email protected] 3.11.6...3.21.4 None +0/-0 colinmcd94

socket-security[bot] avatar May 02 '23 21:05 socket-security[bot]

Hey @solomonhawk, Any news on this PR? Did you find another workaround? I'm also using z.coerce.date in one of my schema and it's not working currently. Thanks!

lildesert avatar Feb 28 '24 15:02 lildesert

@lildesert I don't have any updates here - the PR is offered up for @kiliman to consider. I don't know if they are actively working on remix-params-helper although it looks like a few updates were released at the end of last year.

solomonhawk avatar Feb 28 '24 15:02 solomonhawk

I'm sorry for not getting back to you sooner. I haven't used this package much lately. I have my own fork essentially with a different API.

I really should get back to this and clean it up. Also, if anyone would like to maintain this going forward, that would be great!

kiliman avatar Feb 28 '24 16:02 kiliman

I'm sorry for not getting back to you sooner. I haven't used this package much lately. I have my own fork essentially with a different API.

I really should get back to this and clean it up. Also, if anyone would like to maintain this going forward, that would be great!

It's all good. I know how it goes :) I don't have time to contribute more unfortunately.

I do think it's a worthwhile pursuit to have a way to derive input props from a schema definition (the main thing I used this package for) as it reduces duplication and helps prevent a divergence between the validation rules and the form impl. It's a little disappointing that building on top of zod isn't simpler. That may be unavoidable given how closely it maps to TS, inheriting much of the complexity of the type system itself.

solomonhawk avatar Feb 28 '24 16:02 solomonhawk