fix: ensure transformed values in field-level schemas are used on submit

[mehcode]

🔎 Overview

This PR allows for the transformed value to pass through as the submitted value, when the transformation is declared in a field-level schema in useField (as opposed to a top-level schema declared in useForm). This affects both Zod and Yup.

This is a revision of #4692

• ✅ Many internal types adjusted to allow for a <TInput, TOutput> vs <TValues>
• ✅ Typecheck passes
• ✅ Rebased
• ✅ All existing tests pass
• ✅ New test with zod and a new test with yup added for this behavior

🤓 Code snippets/examples (if applicable)

// in a field component

const testRules = toTypedSchema(z.string().transform(value => value.trim()));
const { value } = useField('test', testRules);

// in a form component / page

const { handleSubmit } = useForm<{ test: string }>();

handleSubmit((values) => {
  console.log(values.test); // logs "hello" (with this PR)
});

With the above field-level usage, on submission of the value " hello " (notice the spaces) with the main branch, the value in values.test is " hello ". Notice the transformation .trim has not been applied.

See the bundled tests for more detail.

✔ Issues affected

Closes #4713

[mehcode]

Please let me know if there is anything else I can do to help get this behavior merged @logaretm. We've been using the other PR in development and it's been working wonderfully. This is a great project you've got here. 🙇 ❤️

[codecov]

Codecov Report

Attention: Patch coverage is 99.24812% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 89.67%. Comparing base (0b219b8) to head (3e292e6).

Files	Patch %	Lines
packages/vee-validate/src/useForm.ts	97.61%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4754      +/-   ##
==========================================
+ Coverage   89.60%   89.67%   +0.06%     
==========================================
  Files          93       93              
  Lines        7765     7814      +49     
  Branches     1370     1374       +4     
==========================================
+ Hits         6958     7007      +49     
  Misses        800      800              
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[logaretm]

Thanks a lot for this, this looks great and many thanks for including tests as well, I'm happy to merge this But I have a few issues and concerns:

• Usage of never, we default to unknown mostly so specifying a type in the first place is a bit redundant, and using never is interesting but I don't think it is correct here. Feel free to correct me.
• Some of type changes around FieldValidator can be considered breaking, It might be okay/necessary, I still need to take another look at them. It may need to default to unknown to fix it for me.
• There is a small conflict with main at the moment, I pushed a change today so you may need to rebase, sorry about that.

I think this is a quality addition and I appreciate the work you've done here! I will be happy to merge it once my issues are addressed.

[mehcode]

I'm happy to give the types another pass. Thank you for taking a look.

As for never, the issue is ValidationResult exists and is both the return type of a public method for field validation and the return type of inside of a list of validations inside of a FormValidationResult.

interface FormValidationResult<TValues, TOutput = TValues> {
    valid: boolean;

    // these are the individual validation results, their values though
    // are collected and expected to be in the top-level `values` object
    results: Partial<Record<Path<TValues>, ValidationResult<never>>>;

    errors: Partial<Record<Path<TValues>, string>>;

    // these are the transformed values as a whole object
    values?: Partial<TOutput>;
}

I used the never concept to tell TypeScript that inside of results the value would not be present. I might have been over-thinking it, maybe its okay if you can look at the transformed result of an individual field in results.

Yeah. It probably makes a lot more sense if we remove never and simply always have the transformed result (if there is one) present in results[key].value.

[mehcode]

@logaretm Please give it another look. I believe I resolved your concerns and I've rebased with main.

[logaretm]

Much better, thank you for the explanation. I think we are getting closer now, just a couple of concerns to address and it should be good to go!

[logaretm]

This might break some people using yup.strip and other kind of object transformations as you are merging the result of the current form values with the parsed ones. Instead of just returning the parsed ones.

I will double check against that tonight. But is it necessary as far as I can tell you are building the value object with a loop anyways:

if (validation.value) {
  setInPath(values, validation.key, validation.value);
}

If not then maybe we can force the validate function on the field-level to always return a value to avoid doing a merge.

[mehcode]

I tried a few different variants of this. I get several errors in tests if I force the field-level validate to always return a value and then do the simple assignment here. Here is one of the more interesting failures:

 FAIL  packages/vee-validate/tests/Form.spec.ts > <Form /> > unmounted fields gets unregistered and their submitted values are kept if configured on the field level
AssertionError: expected last "spy" call to have been called with [ { drink: [ 'Tea', 'Coke' ], …(2) } ]

- Expected
+ Received

  Array [
    Object {
      "drink": Array [
        "Tea",
        "Coke",
      ],
-     "nested": Object {
-       "field": "12",
-     },
-     "non-nested.field": "12",
    },
  ]

[logaretm]

Hey, I tested it against yup.strip and it fails. I added a test case in the main branch and force pushed it to your branch. This is a major blocker. I will try to come up with something but we want to avoid using assign here as my latest review.

[mehcode]

Really appreciate the failing test case. I'll take a look too.

[logaretm]

@mehcode I figured out a way to fix it in 3e292e6 which should be safe to do.

Basically we only want to use the merge strategy when the fields are the source of validation, while use the entire value (re-assign) if the source was a schema. So I added a source property instead on the form result object and made sure it returns in every path.

We toggle between the two behaviors depending on the validation/parse source, I think this is safe to have right now. If all good with you I can merge this and tag a release. What do you think?

[logaretm]

Good to go once I get your opinion.

[mehcode]

This looks like a good solution. Thank you 🙇 for taking the time to get this working.

[logaretm]

Awesome, thanks so much for the work you've put here. I will tag a release tomorrow.