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

[9.x] Adds ability to compiles new http verbs directives 🚦 #45927

Closed
wants to merge 8 commits into from
Closed

[9.x] Adds ability to compiles new http verbs directives 🚦 #45927

wants to merge 8 commits into from

Conversation

mahmoudmohamedramadan
Copy link
Contributor

@mahmoudmohamedramadan mahmoudmohamedramadan commented Feb 2, 2023

This PR allows us to use the new short directives instead @method directive, let's examine the next code for more illumination.

BEFORE

<form method="post" action="/users/{{ $user->id }}">
    @csrf
    @method('PATCH')
</form>

But after this PR gets merged, the code will be like so 🎉

AFTER

<form method="post" action="/users/{{ $user->id }}">
    @csrf
    @patch
</form>

@browner12
Copy link
Contributor

this seems excessive, IMO. we're not gaining anything with the switch, save a few keystrokes.

if this were to get merged, would it have to go to 10.x, in case someone is defining these in userland? I'm not sure the precedence when it comes to Blade.

@timacdonald
Copy link
Member

@browner12 userland directives will take precedence over framework ones. See #28062 (comment)

I like the idea, but wanted to also replace the need for action, method, and patch input in one swoop.

I've managed to do this with the following API:

<!-- BEFORE -->

<form method="post" action="/users/{{ $user->id }}">
    @csrf
    @method('PATCH')
</form>

<!-- AFTER -->

<form @patch("/users/{$user->id}")">
    @csrf
</form>

This adds the _method input into the query parameter instead of the form body.

It is still possible to put it into the method body via @mahmoudmohamedramadan's original API.

No arguments: outputs the hidden input.
Argument: outputs the method and action form attribute.

FYI: a url can have more than one question mark, but the first one always indicates the start of the query parameters, hence the primitive string concatenation is possible without fully parsing into a URL structure.

@mahmoudmohamedramadan
Copy link
Contributor Author

@timacdonald The feature of canceling method and action attributes are very nice but, there is an issue of your code becuase, you send the _method as a query string and it must be sent in the form body.

@mahmoudmohamedramadan
Copy link
Contributor Author

mahmoudmohamedramadan commented Feb 3, 2023

I have updated the code according to the new code style that @timacdonald suggested but, I found some issues, so I have solved these issues and the code style has changed again 🚀.

Before

<form method="post" action="/users/{{ $user->id }}">
    @csrf
    @method('PATCH')
</form>

AFTER

<form @patch("/users/{$user->id}")>
    @csrf
    @patch
</form>

@taylorotwell
Copy link
Member

@mahmoudmohamedramadan are you sure it can't be in the query string. Here is the source:

CleanShot 2023-02-02 at 21 00 30@2x

@timacdonald
Copy link
Member

Here is a quick script to demo the query parameter working.

composer create-project laravel/laravel patch-test \
  && cd patch-test \
  && echo "Route::patch('patch-route', fn () => 'You are on a PATCH route!');" >> routes/web.php \
  && echo "<form method='POST' action='/patch-route?_method=PATCH'>
  @csrf
  <button>Submit</button>
</form>" > resources/views/welcome.blade.php

## Open in browser
(sleep 1 && open http://127.0.0.1:8000/)&
php artisan serve

@mahmoudmohamedramadan
Copy link
Contributor Author

mahmoudmohamedramadan commented Feb 3, 2023

@taylorotwell Whenever I submit the form, the 419 exception is thrown.

@timacdonald
Copy link
Member

Is the form using use the @csrf directive? That is still required.

@mahmoudmohamedramadan mahmoudmohamedramadan changed the title Adds the ability to compiles http verbs methods 🚦 Adds ability to compiles new http verbs directives 🚦 Feb 3, 2023
@timacdonald timacdonald changed the title Adds ability to compiles new http verbs directives 🚦 [9.x] Adds ability to compiles new http verbs directives 🚦 Feb 3, 2023
@taylorotwell
Copy link
Member

Tabling this one for now.

@mahmoudmohamedramadan
Copy link
Contributor Author

@taylorotwell This pull request will never be merged?

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