-
Notifications
You must be signed in to change notification settings - Fork 504
Revamp useForm's generic types across adaptors #2335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This will allow backends to send an array of strings and have the front end have correct types.
42fbe8c adds the ability to override the error type. This was sparked by a request from @RobertBoes to allow an array of strings from the backend, rather than just one, or an mapping of named rules to human descriptions. This allows for greater flexibility for backends. |
Taking a quick look at this, shouldn't the return type of transform be a FormDataType instead of any object as it must be valid at that point to send to the server? Another thing of note, my use case doesn't look like it'll ever be directly supported, which is fine:
Basically making my working data 'invalid' but turning it into something valid for submission. I'm hoping there is an elegant way I can do this without sacrificing all other protections. My first thought would be to allow module augmentation on FormDataConvertible like this:
So consumers can add their own data types that will correctly serialize (or intend to always transform) themselves
This looks like it could be doable with this approach. There might be some better ways, but let me know what you think |
To be blunt, I kinda considered it out of scope, since there is no good way to deal with the non-sense people can get up to with
This above largely why, though, does not full show why. There is nothing stopping someone from changing the shape of the object inside const form = useForm({ bornOn: undefined as Optional<Dayjs> });
form.transform(({ bornOn }) => ({
myDate: bornOn?.format('YYYY-MM-DD'),
}));
form;
// ^? const form: InertiaFormProps<{ bornOn: Optional<Dayjs> }>
form.errors;
// ^? (property) InertiaFormProps<{ bornOn: Optional<Dayjs> }>.errors: Partial<Record<"bornOn", string>>
// Validation errors no longer align with the form's own data
console.log(form.errors)
// We end up with errors in the shape of Partial<Record<"myDate", string>>
If there was a way to force the
That's a fair thought and all, but disagree somewhat. I don't think there should be a global way to register that a complex type is valid in a form without providing a way to register a sort of global handler to guarantee it will serialize correctly without manual intervention every time it's used with transform or something of the like. As this PR was aimed only at pure type level improvements, such a thing is out of scope for this PR in my eyes. As an aside, is there a reason you are so fixed on keeping a DateJS instance inside a form, instead say of a Date or raw string? If I needed a enhanced Date instance (Luxon is my pick for that at least) for display or added validation reasons, I'd be using Vue's |
Thanks for your detailed response. I definitely see your reasoning around the typing of transform. As for my usage of fancy date objects - the primary reason is ergonomics. The date picker library I use will happily take and spit out dayjs instances, and so has every form state library I've used in the past, making it quick to manipulate and format. This being the one exception is kind of a pain. I can see how the design of this library makes it very difficult to safely type, but I feel that module augmentation firmly lies in "I know what I'm doing" territory. Other packages I use such as mui-x rely heavily on this for easy customisation, but very rarely is module augmentation completely "safe", so I don't believe a feature like this would need any guard rails to guarantee anything. As with transform, it can act as an escape hatch to work around specific problems. I know I'm barking up the wrong tree here, but I'd like to hear other opinions before deciding to push the idea. Thanks once again for your efforts. |
Thanks for this, @Spice-King! Just wanted to let you know that I'm reviewing it now, but it might take some time as it's quite complicated 😬 |
Yea, sorry about that @pascalbaljet, TS magic is like that. Will recommend going commit by commit, though a high level summery of the black magic in If you want to pick my brain on any given bit, I am on the Discord, rather than spamming the emails of those who are following. |
What started as a rework to solve allowing otherwise valid interfaces being provided directly via
useForm<SomeInterface>({...})
(fixing #2188), ended up as a total rework of the generic types they use and their uses and expanding on the work I did in #1649. Hell of a nerd snipe on myself thinking it could be solved.All adaptors no longer have their own bespoke but functionally identical
FormDataType
, and now use one that I added to core. It should reject things that are invalid to be in the form (Functions come to mind on the typings, but anything that's not basic JSON data), which might cause some type issues for end users if they were doing something invalid to start with. If this is too tight, it can be made more permissive if that's something people want later.Updated FormDataValues and FormDataKeys to dive down into arrays and TypeScript tuples correctly. I did quickly test all the playgrounds to make sure the adaptors use it for things like
defaults
/setDefault
,setData
/setStore
, orreset
. All of them work correctly even with arrays, with the one rub being thatreset
has a footgun with keys nested inside an array (if a default for the index on the array does not have a value set, it's behaviour is not well defined, ranging from setting the index to undefined or not resetting a value at all).A type for form errors was extracted to core as
Partial<Record<FormDataKeys<TForm>, string>>
everywhere was verbose.Relevant types have been exported on the core package for any third party client adaptors to use.
Demo of types
I expect this is not a breaking type change for either users or third party client adaptors, unless they were technically invalid to start with. I could be wrong though. The form error type might be wrong if a server side adaptor does not follow what Laravel's error bag does for nested data structures (IE:
array.0.key
), but it's not like it was being handled correctly before anyway. This is about two days of head bashing and checking done, but open to people poking holes where they can.