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

BackedEnum on Route already resolved #51583

Closed
CAAHS opened this issue May 27, 2024 · 3 comments
Closed

BackedEnum on Route already resolved #51583

CAAHS opened this issue May 27, 2024 · 3 comments

Comments

@CAAHS
Copy link
Contributor

CAAHS commented May 27, 2024

Laravel Version

10.48.11

PHP Version

8.1.28

Database Driver & Version

No response

Description

Given the $parameterValue on the route is already resolved, do not try to resolve it from stringable any longer.

Otherwise an error (current behavior):

 Object of class App\Models\StringBackedEnum could not be converted to string

Error occured first during an update of livewire/livewire:

Production Changes From To Compare
livewire/livewire v3.3.0 v3.3.1 ...

(isolated change, from livewire/livewire:v3.3.1...latest the error retains)

$ artisan --version
Laravel Framework 10.48.11

We have a trivial patch in \Illuminate\Routing\ImplicitRouteBinding::resolveBackedEnumsForRoute:

diff --git a/vendor/laravel/framework/src/Illuminate/Routing/ImplicitRouteBinding.php b/vendor/laravel/framework/src/Illuminate/Routing/ImplicitRouteBindingB.php
index d3590de..04921e0 100644
--- a/vendor/laravel/framework/src/Illuminate/Routing/ImplicitRouteBinding.php
+++ b/vendor/laravel/framework/src/Illuminate/Routing/ImplicitRouteBindingB.php
@@ -86,7 +86,13 @@ class ImplicitRouteBinding
 
             $backedEnumClass = $parameter->getType()?->getName();
 
-            $backedEnum = $backedEnumClass::tryFrom((string) $parameterValue);
+            $backedEnum = null;
+
+            if ($parameterValue instanceof $backedEnumClass) {
+                $backedEnum = $parameterValue;
+            }
+
+            $backedEnum = $backedEnum ?? $backedEnumClass::tryFrom((string) $parameterValue);
 
             if (is_null($backedEnum)) {
                 throw new BackedEnumCaseNotFoundException($backedEnumClass, $parameterValue);

and do see this is induced by Livewire.

From the type of change we believe this is a good fit for supporting (String)BackedEnums on Routes.

Please let us know if this is not too far off, then we would create a pull request.

Please share your recommendations.

Steps To Reproduce

  • Wire a route with a backed enum parameter

  • Inject the enum, directly or indirectly (e.g. use Livewire)

  • Error throws at vendor/laravel/framework/src/Illuminate/Routing/ImplicitRouteBinding.php:89

    $backedEnum = $backedEnumClass::tryFrom((string) $parameterValue);
                                                    ^
                                                    |
               /!\ Object of class App\Models\StringBackedEnum could not be converted to string
    
@crynobone
Copy link
Member

Should be able to backport #51525 to 10.x branch

Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

CAAHS pushed a commit to CAAHS/laravel-framework that referenced this issue May 27, 2024
Port a078b1a (Fixes explicit route binding with `BackedEnum` (laravel#51525),
2024-05-21 crynobone) to 10.x (from 11.x).

fixes laravel#51583
refs laravel#51514

Signed-off-by: CAAHS <[email protected]>
@CAAHS
Copy link
Contributor Author

CAAHS commented May 27, 2024

Should be able to backport #51525 to 10.x branch

Thanks for the pointers. Looks like the exact same change. We were able to file a PR or two, this is the open one: #51586 @crynobone

CAAHS pushed a commit to CAAHS/laravel-framework that referenced this issue May 27, 2024
Port a078b1a (Fixes explicit route binding with `BackedEnum` (laravel#51525),
2024-05-21 crynobone) to 10.x (from 11.x).

fixes laravel#51583
refs laravel#51514

Signed-off-by: CAAHS <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants