romsar / gonertia

The Go adapter for Inertia.js
https://pkg.go.dev/github.com/romsar/gonertia
Other
88 stars 4 forks source link

Using Inertia.Back() with error lets results in useForm() not having errors #8

Closed JensvandeWiel closed 4 months ago

JensvandeWiel commented 4 months ago

When i add errors to context and redirect back the useFrom().errors is empty while when i check usePage().props.errors it is there.

I think this is because the request made to the server returns a 409 code which results in a full page reload resulting in thet useForm() does not control that requests anymore, because inertia.Back() does this.

As i've seen in laravel when validation errors this does not happen, it returns a 302 status back to the page, just like i want to do with inertia.Back(). After this redirect back it updates the page without full reloading

JensvandeWiel commented 4 months ago

I know that inertia.Back() must return 409 but there should be a function or middleware to handle validation errors more nicely. Like a function redirectWithErrors that flashes the errors and then redirect back and let inertia reload using xhr.

romsar commented 4 months ago

Hmmm, any ideas why laravel returning 302 when we can see in the code that it's should always be 409? https://github.com/inertiajs/inertia-laravel/blob/1.x/src/ResponseFactory.php#L112-L114

I need to dig into this a little more.

JensvandeWiel commented 4 months ago

I still don't know why, i only know after validation that it doesnt. This is what I've made so far and works:

func (h *LoginInertiaHandler) ProcessLogin(ctx echo.Context) error {
    err := flash.GetFlashProvider(ctx).FlashErrors(ctx.Request().Context(), gonertia.ValidationErrors{"email": "Invalid email address"})
    if err != nil {
        return err
    }
    http.Redirect(ctx.Response(), ctx.Request(), "/login", http.StatusSeeOther)
    return nil
}

So for maximum control it would be nice to manually flash errors in these cases, altough this can lead to unexpected behaviour so a function like inertia.RedirectWithErrors and/or inertia.RedirectWithErrorsFromContext

If you want i can make a quick pull request for this.

JensvandeWiel commented 4 months ago

@romsar I know why, in Laravel if validation fails it flashes the errors to the storage and then redirects back, inertia is then smart enough to get the errors from Laravels flash storage and add them as props.

romsar commented 4 months ago

I'm just thinking, what if we check for errors before redirecting? If there is at least one, then we will give the code 302, not 409.

JensvandeWiel commented 4 months ago

I talked to robert boes in the inertia discord. And the Location function should indeed return 409. So there should be another function to redirect with errors

JensvandeWiel commented 4 months ago

He said Location() and Back() are not for this use case

What, I don't understand? The location response has a different purpose.. you wouldn't really use that in any normal use case. Especially not if it's just a redirect to login

romsar commented 4 months ago

I think I originally used Back incorrectly, it should use 30x redirect instead of calling Location. After a little thought, it seems to me that we should make new method Redirect and rewrite the Back. Both of them will flash errors into session.

romsar commented 4 months ago

At first I thought towards middleware, but it seems that we do not have access to the context that we pass to http.Redirect.

JensvandeWiel commented 4 months ago

Yep, thats a great idea. Cant wait for it to be added :)

romsar commented 4 months ago

Thanks for noticing the problem! Please check version v1.2.2. In addition, I have updated the readme.

romsar commented 4 months ago

@JensvandeWiel can we close this issue?

JensvandeWiel commented 4 months ago

Sure