Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add user(): helper function#6582

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
imliam wants to merge3 commits intolaravel:12.x
base:12.x
Choose a base branch
Loading
fromimliam:user-helper
Draft

Conversation

imliam
Copy link

@imliamimliam commentedMar 15, 2025
edited
Loading

This adds auser() helper function to the base application, which is type-hinted to return an instance of theApp\Models\User class by default.

Why?

auth()->user() is handy, and widely used in Laravel projects, but its return typehint is for aIlluminate\Contracts\Auth\Authenticatable instance.

While this may be valid from a framework perspective, and there are many uses for Authenticatables that are not the User model, most Laravel applications use the defaults. Therefore, users expect an instance of the User model returned.

This means that, without the help of third-party tools likelaravel-ide-helper, as developers we get a poor experience when getting the authenticated user and expecting the model:

  • No IDE support for chaining methods/properties
  • Static analysis fails if we chain anything not on the Authenticatable interface
  • We have to typehint/** @var \App\Models\User $user */ everywhere we use the helper

By adding this to the basic application scaffold, the newuser() helper can get proper typehinting for the out-of-the-box User model that ships with a fresh application, supporting proper typehinting. As it is in the userland instead of the framework, it can be changed to support other Authenticatables if the developer wishes.

Why should this be in the base application?

Yes, users could always add this helper manually to their app, but having a standard way the framework does it will eventually bring consistency to new applications—and I believe this utility is used commonly enough that it warrants being in the base.

There's clearlysome interest in it, and people have a dozen of their own home-grown solutions for the same problem, something this could provide.

Why not in the framework?

App\Models\User is defined in the user's application, so the framework has no knowledge of it or if it changes names/namespaces/etc.

ducduy2424 reacted with thumbs up emojiJames4645, aneesdev, saeedvaziry, MusahMusah, AhmedAlaa4611, laserhybiz, danie-ramdhani, balu-lt, frans-slabbekoorn, hassam-labs, and 35 more reacted with thumbs down emojiCharrafiMed, studnitz, datlechin, dannewnsjump24, damms005, amirtavakolian, appsaeed, Naoray, realazaber, kippenator, and 5 more reacted with heart emojidcblogdev, CharrafiMed, damms005, and parsa-mostafaie reacted with rocket emoji
@James4645
Copy link

James4645 commentedMar 16, 2025
edited
Loading

I don't get the point of this. Seems like yet another worthless file I'll have to remove from every app I make like the console routes file. And seems directly contrary to th direction Laravel went with 11.x for slimming the skeleton.

If you want it just add it yourself to your app.

Motifsky, mateusjunges, saeedvaziry, badasukerubin, AhmedAlaa4611, laserhybiz, frans-slabbekoorn, stevenobird, masatokawasakii, conghyu, and 16 more reacted with thumbs up emojibenjamimWalker reacted with heart emoji

@joshmanders
Copy link
Contributor

Yeah this should be either apart of the framework's existing helpers or added yourself to your own project.

With the new--using flag in the installer you can make your own starter with this

mostafaznv reacted with thumbs up emoji

@taylorotwelltaylorotwell marked this pull request as draftMarch 16, 2025 23:51
@AhmedAlaa4611
Copy link
Contributor

We already have Laravel's default middlewareauth andguest, this is useless.

dcgitx, frans-slabbekoorn, MrVH-IR, AhmedAlaa4611, ersin-demirtas, rokasma, joke2k, shaedrich, martinbean, mostafaznv, and noxsii reacted with thumbs up emojicarlosmintfan, adevade, realazaber, Muetze42, egyjs, danharrin, abcmoritz, and goodway reacted with thumbs down emoji

thrownewException('The current user is not authenticated.');
}

if (!$currentUserinstanceof User) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As the function has a return type ofApp\Models\User this check is unnecessary.

If an object which is not an instance ofApp\Models\User is returned, PHP will error out automatically.

nozkok, balu-lt, stevenobird, and abcmoritz reacted with thumbs up emoji
Copy link
Author

@imliamimliamMar 18, 2025
edited
Loading

Choose a reason for hiding this comment

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

It would throw a TypeError you're right, but I made it explicit here so that static analysis tools can tell what's going on

Another option would be to docblock the$currentUser variable for static analysis and allow the TypeError to be thrown at runtime

Copy link
Contributor

Choose a reason for hiding this comment

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

We already have the user model defined in the config:auth.providers.users.model.
It should use that.

I’m experiencing the same frustration with PHPStan. I’ll probably implement something like this in my projects, but I feel like a better version should be implemented at the framework level.

rodrigopedra reacted with thumbs up emoji
@rodrigopedra
Copy link
Contributor

I usually add ahelpers.php file to my app's./app folder and namespace it onApp, something like this:

<?phpnamespaceApp;functionfoo() {}

It is still short enough to use in views ({{ \App\foo() }}), and in regular code I can import it. And has the benefit of not polluting the global namespace.

I would vote to have the helpers file under./app, as it is application's code.

I can see the benefit of having this file included on the likes to show users how easy, and useful, it is to have a helpers file.

Much like what we have in./route/console.php, which ships with theinspiring command, and anyone who doesn't want that command can easily remove it on their app.

designgears reacted with thumbs up emoji

@msamgan
Copy link

It also has the additional benefit of an added helper file that will come with the project, most of the devs had to add that file to their project manually. Maybebootstrap/ is not the right place, but the idea in itself is not bad.

peterjthomson, masatokawasakii, msamgan, and kippenator reacted with thumbs up emojifrans-slabbekoorn, osbre, inmanturbo, Muetze42, and goodway reacted with thumbs down emoji

thrownewException('The current user is not authenticated.');
}

if (!$currentUserinstanceof User) {

Choose a reason for hiding this comment

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

Helpful idea, but I think usingApp\Models\User directly could be limiting. In many cases, the authenticated user isn't always aUser instance — it can be anyAuthenticatable implementation, defined viaauth.providers.users.model and possibly located in a different namespace.

Also, when callingauth()->user() (or this helper if added), developers typically expect anAuthenticatable, not specifically aUser model. This might lead to confusion or unexpected type errors in custom setups.

philkobby, martinbean, ImraneAabbou, nickfls, sumiredc, francoism90, and frans-slabbekoorn reacted with thumbs up emoji

if (!$currentUserinstanceof User) {
thrownewException('The currently authenticated user is not an instance of'.User::class);
}

Choose a reason for hiding this comment

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

This breaks in apps with multiple auth guards. It assumes only User gets authenticated.

joke2k, AhmedAlaa4611, KhairulAzmi21, crazynds, frans-slabbekoorn, martinbean, entense, el-doksh, and francoism90 reacted with thumbs up emoji
@MR-Sharifi
Copy link

In my opinion, it's not a necessary functionality. Even if it was, it does not consider all possible situations. For example, what if someone (like me) uses a custom-curated class for the user entity that does not extend the App\Models\User?

AhmedAlaa4611, datamweb, shaedrich, and goodway reacted with thumbs up emoji

$currentUser =auth()->user();

if ($currentUser ===null) {
thrownewException('The current user is not authenticated.');

Choose a reason for hiding this comment

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

Maybe, this could be a dedicatedUnauthenticatedException or the like 🤔

a-h-abid and papystanny reacted with thumbs up emoji
Comment on lines +12 to +14
if ($currentUser ===null) {
thrownewException('The current user is not authenticated.');
}

Choose a reason for hiding this comment

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

If you want, you could shorten this

Suggested change
if ($currentUser ===null) {
thrownewException('The current user is not authenticated.');
}
throw_if($currentUser ===null, Exception::class,'The current user is not authenticated.');

noxsii reacted with thumbs up emoji
@inmanturbo
Copy link

inmanturbo commentedApr 18, 2025
edited
Loading

I think this is an issue for the vscode extension honestly. There has got to be a good way somehow to configure static analysis to bind preferred, current or default implementations for contracts in general.

I honestly feel the OP's pain. I do, but this is part of a larger problem.Authenticatable is one of many cross cutting, integral contracts provided and fulfilled by the framework. If only we could globally configure the ones we're most concerned with somehow using some kind of config map? Preferably not in yaml. 😅

shaedrich and designgears reacted with thumbs up emoji

@RyanPaiva56
Copy link

Agree that this is necessary. Also agree that improving the IDE extension is the way to go. Laravel needs a best-in-class IDE that works out of the box without having to install a bunch of things to get it to work.

Now that Laravel Cloud exists, the IDE is the last painful point that sucks for literally every developer. PHPStorm wont do anything and defers everything to the IDEHelper (BLESS BARRYV) but it adds a ton of bloat.

VSCode is the obvious choice and the official extension is really the future. We shouldn't be making framework choices based on IDEs being bad at what they do. Instead we should be improving the IDE extension.

@rodrigopedra
Copy link
Contributor

PHPStorm wont do anything and defers everything to the IDEHelper (BLESS BARRYV) but it adds a ton of bloat.

The Laravel IDEA plugin is worth every cent, and works incredible well with (most) type analysis needed on a Laravel project.

PHP Storm offers the Laravel IDEA plugin at a discounted rate when licensing both the IDE and the plugin together.

I am not affiliated to PHP Storm nor to the Laravel IDEA plugin, just a satisfied client of both tools

Usmanzahidcode and stevenobird reacted with thumbs up emoji

@RyanPaiva56
Copy link

PHPStorm wont do anything and defers everything to the IDEHelper (BLESS BARRYV) but it adds a ton of bloat.

The Laravel IDEA plugin is worth every cent, and works incredible well with (most) type analysis needed on a Laravel project.

PHP Storm offers the Laravel IDEA plugin at a discounted rate when licensing both the IDE and the plugin together.

I am not affiliated to PHP Storm nor to the Laravel IDEA plugin, just a satisfied client of both tools

I've paid for both and went to vscode anyway. Caleb's make vscode awesome already puts it ahead of phpstorm, and with intelephense (the paid version) and the Laravel vscode extension, it's miles ahead. If we can invest more in a 1st party extension, it solves all of these issues. Case in point, we have talented developers sending pull requests for functions that have been around from day 1 because IDEs are still painful even for veterans.

AhmedAlaa4611 reacted with thumbs up emoji

@benfaerber
Copy link

benfaerber commentedApr 23, 2025
edited
Loading

I always create a method calledUser::auth() on my user to get proper hints.

class User {publicstaticfunctionauth():self {$user =auth()->user();throw_if(!$user, Exception::class,"Called from invalid auth context!");return$user;    }}
donald-ov and ct1735x reacted with thumbs up emojishaedrich and balu-lt reacted with eyes emoji

@shaedrich
Copy link

shaedrich commentedApr 23, 2025
edited
Loading

I always create a method calledUser::auth() on my user to get proper hints.

This might be interesting to add to\Illuminate\Auth\Authenticatable, however, that won't solve@imliam's original problem 🤔

benfaerber reacted with thumbs up emoji

@nazmulpcc
Copy link

Ignoring other issues, this will need manual tweaking as soon as someone uses another Authenticatable class.
What I do is add something like this in a.ide.php file

namespaceIlluminate\Contracts\Auth {interface Guard    {publicfunctionuser():\App\Models\User;    }}

I don't usually reach forbarryvdh/laravel-ide-helper in simple cases like this.

francoism90 reacted with thumbs up emoji

@Aluisio-Pires
Copy link
Contributor

I think your idea is excellent, but the way it’s been implemented doesn’t align with the framework’s current structure. Depending on the project, we might have multiple authentication models or models organized into separate folders in modular architectures. The helper should be flexible enough to handle these cases. If you can refine your approach, it would make a wonderful contribution ❤️🚀

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

@rodrigopedrarodrigopedrarodrigopedra left review comments

@balu-ltbalu-ltbalu-lt left review comments

@shaedrichshaedrichshaedrich left review comments

@lucas-or-notlucas-or-notlucas-or-not left review comments

@UsmanzahidcodeUsmanzahidcodeUsmanzahidcode left review comments

Copilot code reviewCopilotAwaiting requested review from CopilotCopilot will automatically review once the pull request is marked ready for review

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

16 participants
@imliam@James4645@joshmanders@AhmedAlaa4611@rodrigopedra@msamgan@MR-Sharifi@inmanturbo@RyanPaiva56@benfaerber@shaedrich@nazmulpcc@Aluisio-Pires@balu-lt@lucas-or-not@Usmanzahidcode

[8]ページ先頭

©2009-2025 Movatter.jp