lorisleiva / laravel-actions

⚡️ Laravel components that take care of one specific task
https://laravelactions.com
MIT License
2.52k stars 124 forks source link

Cleanup docblocks added return types #285

Closed edwinvdpol closed 3 months ago

edwinvdpol commented 3 months ago

Hi there,

Don't be shocked by the amount of files changed, but I felt like doing the following:

There were quite a few return types missing in the test files, that's why it looks so much.

Hope you like it, and feel free to contact me if you have any questions / remarks. You are free to close this PR if you don't like it of course, but static analysis tools will be (more) happy eventually. 😉

👋 Edwin

Casmo commented 3 months ago

Since this chance I have the following error when running this:

$accessToken = GetJobApiToken::run();

Error: App\Domain\Utility\Actions\GetJobApiToken::make(): Return value must be of type App\Domain\Utility\Actions\GetJobApiToken, Lorisleiva\Actions\Decorators\ListenerDecorator returned {"exception":"[object] (TypeError(code: 0): App\Domain\Utility\Actions\GetJobApiToken::make(): Return value must be of type App\Domain\Utility\Actions\GetJobApiToken, Lorisleiva\Actions\Decorators\ListenerDecorator returned at /.../ami-premium/vendor/lorisleiva/laravel-actions/src/Concerns/AsObject.php:11)

This error is (only) showned when the running the code above from a Job it seems. Not directly by calling GetJobApiToken::run().

Reverting this line and the issue is resolved: https://github.com/lorisleiva/laravel-actions/pull/285/files#diff-83b0bf013cf9ca67a5744e4b8f26726490d9b9df77df9a1f78036397ff085a14R9

lorisleiva commented 3 months ago

Oh yeah, I guess that's because the resolved instance may not be an instance of AsObject.

Would you mind submitting a small PR for this change? 🙏

RomanNebesnuyGC commented 2 months ago

@edwinvdpol Hi)

But now navigating by method not working correctly at PhpStorm for example.

Before make()->handle() goes to handle in Action class. Now because of removed comments at AsObject trait it goes to handle() at Illuminate/Foundation/Application.php and code is visually broken.

Do we really need this (removing comments)?

lorisleiva commented 2 months ago

@RomanNebesnuyGC See https://github.com/lorisleiva/laravel-actions/pull/287#issuecomment-2315208593. Feel free to submit a small PR and I'll merge/publish today. 🙏