Skip to content

Conversation

@panakour
Copy link
Contributor

@panakour panakour commented Jan 9, 2025

Replace string class names with ::class constants to be consistent with the rest of the codebase.

This change:

  • Improves type safety
  • Makes refactoring easier
  • Maintains consistency with Laravel's coding style
@taylorotwell taylorotwell merged commit aed6bbe into laravel:11.x Jan 9, 2025
40 checks passed
@SanderMuller
Copy link
Contributor

@taylorotwell I think this is a breaking change on the old route namespacing eg

Route::namespace('Admin') ->group(function (): void { Route::view('dashboard', 'admin.dashboard'); }); 

this now throws Invalid route action: [App\Http\Controllers\Admin\Illuminate\Routing\ViewController]. because the namespace in this PR was changed from \Illuminate\Routing\ViewController to Illuminate\Routing\ViewController I think.

image

@GrahamCampbell
Copy link
Collaborator

The important difference is the leading slash is not included in the ::class name.

taylorotwell pushed a commit that referenced this pull request Jan 14, 2025
…::class constants" (#54185) * Revert "refactor from string class names to constant (#54134)" This reverts commit aed6bbe. * Add failing test for #54185
@joshmanders
Copy link
Contributor

The important difference is the leading slash is not included in the ::class name.

That's interesting, wouldn't it make sense to prefix with \ when calling ::class internally to make sure it's the fully qualified class name?

@shaedrich
Copy link
Contributor

@joshmanders I had the same idea:
#54185 (comment)

@SanderMuller
Copy link
Contributor

The important difference is the leading slash is not included in the ::class name.

That's interesting, wouldn't it make sense to prefix with \ when calling ::class internally to make sure it's the fully qualified class name?

Made an attempt to reintroduce it via #54189

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants