logaretm / vee-validate

✅ Painless Vue forms
https://vee-validate.logaretm.com/v4
MIT License
10.84k stars 1.27k forks source link

Types problem with different path syntax #4295

Open bgoscinski opened 1 year ago

bgoscinski commented 1 year ago

What happened?

The path syntax that works with form errors generated by validation schemas is rejected by form.setFieldValue typechecking. This only affects TS usage because if I silence the TS error form.setFieldValue works as expected using both syntaxes.

Can the Path type be updated so that it produces paths like this:

type Step = {
  name: string
}

type Form = {
  steps: Step[]
}

type Result = Path<Form> // `steps` | `steps[${number}]` | `steps[${number}].name`
//                                          ^         ^          ^         ^

// instead of
type Result = Path<Form> // `steps` | `steps.${number}` | `steps.${number}.name`
//                                          ^                   ^         

Reproduction steps

  1. Visit https://stackblitz.com/edit/vee-validate-v4-cross-field-validation-rfx97g?file=src%2Fmain.ts
  2. See the TS error at line 39 of main.ts
  3. See the error message displayed only for steps[0].name in rendered app side

Version

Vue.js 3.x and vee-validate 4.x

What browsers are you seeing the problem on?

Relevant log output

No response

Demo link

https://stackblitz.com/edit/vee-validate-v4-cross-field-validation-rfx97g?file=src%2Fmain.ts

Code of Conduct

mohanadkandil commented 1 year ago

Can I take this, if no one is working on it ?

logaretm commented 1 year ago

@mohanadkandil Be my guest, basically, we need to allow both bracket and dot syntax to reference arrays.

mohanadkandil commented 1 year ago

Is there any other place other than this one ArrayPathImpl. I just handled this case like the one mentioned here

true extends AnyIsEqual<TraversedTypes, V>
    ? never
    : K extends `${infer Index}.[${infer Rest}]`
    ? Index extends keyof V
      ? Rest extends ArrayPath<V[Index]>
        ? ArrayPathImpl<Rest, V[Index], TraversedTypes | V>
        : never
      : never
    : `${K}` | `${K}.${ArrayPathInternal<V, TraversedTypes | V>}`
  : true extends AnyIsEqual<TraversedTypes, V>
  ? never
  : `${K}.${ArrayPathInternal<V, TraversedTypes | V>}`;

I'll be exploring the rest of the code related to this issue, if there's any other edge case

logaretm commented 1 year ago

@mohanadkandil Create a PR and we can discuss it there, should have a couple of tests as well.

Basically, we want of these expressions to work:

setFieldValue('some.path.array[0].prop');
setFieldValue('some.path.array.0.prop');
setFieldValue('some.other.path.array[0][1]');
setFieldValue('some.other.path.array.0.1');
benax-se commented 1 year ago

@logaretm i have the same issue accesing the error field array[idx].field is working in runtime but typescript allows only array.idx.field

unshame commented 11 months ago

@logaretm what's the use case for supporting array.number access alongside array[number]?

I have a reworked Path type that I can submit as a PR, but I need to understand if the dot notation actually works in some cases or if the type is just incorrect.

logaretm commented 11 months ago

@unshame It's just easier in cases like somefield.1234sike will be treated like an object where somefield[1234sike] would crash or it means we must handle it. It also makes mapping errors/values easier since we can follow a dot notation path instead of keeping track of the [ we have opened.

If you already have something ready feel free to submit it and I can test it out and see how it goes.

I was thinking of completely deprecating [] as an allowed character in the path syntax, so people would be forced to use the dot paths all the time. (Major breaking change of course)

unshame commented 11 months ago

I was thinking the opposite: deprecating the dot notation in favor of brackets 😅

The problem is the errors object only works with the bracket notation, which means the dot notation is basically unusable with arrays if you want to show the error message (which is always).

So as far as I can see, these are the options:

  1. The types should accept either notation. The errors object needs to be fixed to work with both notations (via a Proxy for example). This is potentially a non-breaking change but I don't know how a custom proxy object would play with the reactivity system
  2. The types are unchanged. The error object must be changed to have the dot notation. This is a breaking change.
  3. The types should only accept the bracket notation. The error object is unchanged. This is a breaking change type-wise only.
  4. The types accept either notation for everything but the errors object. The errors object is unchanged. This is a non-breaking change but it also preserves the bug where the error message doesn't work with the dot notation.

Of course there's also this syntax to consider https://vee-validate.logaretm.com/v4/guide/components/nested-objects-and-arrays/#avoiding-nesting With that I'm not sure how deprecating brackets in paths would even work. It also seems to not be supported on the type level currently.

heohex commented 1 month ago

@logaretm Any update on this?

logaretm commented 1 month ago

@heohex No progress on this yet. I can't find the time to address it. In v5 we would remove one syntax to avoid this completely.