Skip to content
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

Fix Interia based useForm return types #55

Closed
wants to merge 2 commits into from

Conversation

Spice-King
Copy link

This will give us usable types for useForm for React and Vue 3 when using Inertia. Related to #31, but not dependant on it.

For now, builds will fail for Vue until inertiajs/inertia#1734 is merged and released, but React will work out of the box. If one wants to test it locally, you can manually fix @inertiajs/vue3 for Typescript's particularities by editing @inertiajs/vue3/types/useForm.d.ts to export it's InertiaFormProps.

Requires inertiajs/inertia#1734 to be published to a new version.
@timacdonald
Copy link
Member

@Spice-King Unfortuantely more work would be required, as the typescript compiler is not happy with these changes. Until it is we can't merge this one. Do you think you would take a look at resolving the issues to get this one working with the compiler?

This is on my list to look at, as I think it will involve a bit more work, but I haven't been able to get to this one yet.

I'll mark as draft until we have a passing build here.

@timacdonald timacdonald marked this pull request as draft November 23, 2023 06:16
@Spice-King
Copy link
Author

Unfortuantely more work would be required, as the typescript compiler is not happy with these changes. Until it is we can't merge this one. Do you think you would take a look at resolving the issues to get this one working with the compiler?

This is on my list to look at, as I think it will involve a bit more work, but I haven't been able to get to this one yet.

@timacdonald there is no further work inside the precognition repo needed, past probably bumping the peer dependency of Inertia to the version which would contain my very minor open PR in inertiajs/inertia#1734. It's unfortunate that Typescript even has error TS4023 in this situation, but at least working around it is easy enough.

I'll mark as draft until we have a passing build here.

I just don't want this to get closed while waiting for upstream. This PR exists so that Inertia can see there is a downstream need for the PR I posted for Inertia, while also fixing the blocker I have to using Precognition with Inertia.

That said, I found a way to trick the Typescript compiler to shut up about it for now here in Precognition. It's less than ideal, but far less ugly than my original attempts to get around it before. I'll toss it into it's own commit so it can be reverted later after Inertia includes my small PR.

@Spice-King Spice-King marked this pull request as ready for review November 23, 2023 16:04
@taylorotwell taylorotwell marked this pull request as draft November 23, 2023 16:19
@timacdonald
Copy link
Member

@Spice-King I'll circle back to this one shortly.

@illusi03
Copy link

Really appreciate your pull request. But however, there are some differences in the types section between Laravel Cognition and Inertia form helpers. One of them is in the code snippet:

// From Precognition
touch(field: keyof TForm): void;
touched(field: keyof TForm): boolean;
valid(field: keyof TForm): boolean;
invalid(field: keyof TForm): boolean;
validate(field: keyof TForm): void;
validateFiles(): void;
validating: boolean;
validator(): void;
setValidationTimeout(duration): void;
forgetError(field: keyof TForm): void;

Let's discuss and align the types for a seamless integration. If you have any questions or suggestions regarding this matter, please feel free to share them. Thank you.

@timacdonald
Copy link
Member

I'm going to close this one off. There is some work on the Inertia side that we would need to make this one viable.

@timacdonald timacdonald closed this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants