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

[5.x] Add support for $view parameter closure with Route::statamic() #11452

Merged
merged 19 commits into from
Feb 20, 2025

Conversation

jesseleite
Copy link
Member

@jesseleite jesseleite commented Feb 13, 2025

This PR adds support for optional $view parameter closure with Statamic::route(), so that you can dynamically render a view from a route segment.

For example, this works fine in Laravel...

Route::get('/components/{component}', function ($component) {
    return view($component);
});

So this should work with Statamic routes as well...

Route::statamic('/components/{component}', function ($component) {
    return view($component);
});

This PR makes it work like this...

image

TODO

  • Support closure in second $view parameter
  • Support closure in third $data parameter
    • This was already supported, but we just want to make sure we don't regress here
  • Support type-hinted service-container-friendly dependency injection like Laravel does
  • Throw helpful exception if they try to use both closures
    • This wouldn't make sense. If you're returning view($template, $data) in Laravel, it's expected you'll pass your dynamic data directly into that 👍
  • Add test coverage
  • Add docs

References #9953

@jesseleite jesseleite marked this pull request as draft February 13, 2025 17:38
@jesseleite jesseleite marked this pull request as ready for review February 14, 2025 22:08
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

The only thing I'd want to change is to make sure if you use Request $request as the first argument of the closure that it'd work too, like a regular route.

Route::statamic('foo/{bar}', function ($bar) {
  return view('myview', ['bar' => $bar];
});

Route::statamic('foo/{bar}', function (Request $request, $bar) {
  return view('myview', ['bar' => $bar];
});

(Maybe it already does work, but a test to show it would be good.)

@jesseleite jesseleite marked this pull request as draft February 14, 2025 22:56
@jesseleite jesseleite marked this pull request as ready for review February 18, 2025 21:38
@jesseleite jesseleite marked this pull request as draft February 19, 2025 17:24
@jesseleite
Copy link
Member Author

jesseleite commented Feb 20, 2025

@jasonvarga After some further testing, technically Request $request doesn't have to come first with Route::get().

Also, Laravel supports and documents type-hinted service-container-friendly dependency injection in general.

I figured we might as well go for feature parity with Route::get(), so now this works 🎉

Route::statamic('example/{one}', function ($one, Request $request, Joe $joe) {
    ray($one, $request, $joe);
});

@jesseleite jesseleite marked this pull request as ready for review February 20, 2025 00:14
@jasonvarga jasonvarga dismissed their stale review February 20, 2025 14:35

Change was made

@jasonvarga jasonvarga merged commit 4ace966 into 5.x Feb 20, 2025
22 checks passed
@jasonvarga jasonvarga deleted the statamic-route-view-closure branch February 20, 2025 14:36
@jasonvarga jasonvarga changed the title [5.x] Add support for $view parameter closure with Statamic::route() [5.x] Add support for $view parameter closure with Route::statamic() Feb 20, 2025
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.

2 participants