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

Add "always" props using new Inertia::always() wrapper #627

Merged
merged 6 commits into from
May 25, 2024

Conversation

lepikhinb
Copy link
Contributor

@lepikhinb lepikhinb commented May 25, 2024

Introduce Inertia::always() wrapper to replace the current persistent props implementation.

return [
    'flash' => Inertia::always(session('flash')),
];

Lazy data evaluation behavior:

return Inertia::render('Users/Index', [
    // ALWAYS included on first visit...
    // OPTIONALLY included on partial reloads...
    // ALWAYS evaluated...
    'users' => User::get(),

    // ALWAYS included on first visit...
    // OPTIONALLY included on partial reloads...
    // ONLY evaluated when needed...
    'users' => fn () => User::get(),

    // NEVER included on first visit...
    // OPTIONALLY included on partial reloads...
    // ONLY evaluated when needed...
    'users' => Inertia::lazy(fn () => User::get()),
    
    // ALWAYS included on first visit...
    // ALWAYS included on partial reloads...
    // ALWAYS evaluated...
    'users' => Inertia::always(User::get()),
]);

@lepikhinb lepikhinb marked this pull request as ready for review May 25, 2024 06:02
src/Middleware.php Outdated Show resolved Hide resolved
src/ResponseFactory.php Outdated Show resolved Hide resolved
@lepikhinb
Copy link
Contributor Author

@reinink made AlwaysProp accept any values including callable 👌🏻

@lepikhinb lepikhinb requested a review from reinink May 25, 2024 20:06
src/Inertia.php Outdated Show resolved Hide resolved
@reinink reinink changed the title Always props Add "always" props using new Inertia::always() wrapper May 25, 2024
@reinink
Copy link
Member

reinink commented May 25, 2024

@lepikhinb Hey thanks for putting this together! 💪

@reinink reinink merged commit 5cbaeab into inertiajs:1.x May 25, 2024
26 checks passed
@romsar romsar mentioned this pull request Jun 12, 2024
@gabrielrbarbosa
Copy link

@lepikhinb "errors" prop is empty with laravel validation after this update

'errors' => function () use ($request) {
return $this->resolveValidationErrors($request);
},
'errors' => Inertia::always($this->resolveValidationErrors($request)),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a breaking change, isnt it? @lepikhinb

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be

@datlechin
Copy link
Contributor

datlechin commented Jun 22, 2024

After updating to v1.3.0, no errors are included in the response; it is always empty.

This issue did not occur in v1.2.0.

@reinink, could you please look into this?

@lepikhinb
Copy link
Contributor Author

@datlechin @gabrielrbarbosa I couldn't reproduce the issue. Is there a chance you can make a quick reproduction?

@datlechin
Copy link
Contributor

@lepikhinb Create a route that performs data validation. Send a request to this route with invalid data and verify that the response does not contain any error messages.

@lepikhinb
Copy link
Contributor Author

@datlechin Please make a repo with a complete reproduction.

Here's PingCRM with Inertia 1.3.

CleanShot 2024-06-22 at 18 36 43@2x

@datlechin
Copy link
Contributor

datlechin commented Jun 23, 2024

Ah i see that what happened

Previously, I used the appendTo method to add Inertia’s middleware, which was works with versions prior to v1.3.0:

->withMiddleware(function (Middleware $middleware) {
    $middleware->appendTo(HandleInertiaRequests::class);
})

After updating to v1.3.0, the approach needs to be changed. Now, you must use the web method to add Inertia’s middleware.

@gabrielrbarbosa
Copy link

@lepikhinb I found out what happened here. We had previously overridden the Response class which made this new update incompatible, I will handle this here or just fix at 1.2.0 for now, thanks anyway!

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.

4 participants