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

Remove return type for make() #287

Merged
merged 3 commits into from
Aug 20, 2024
Merged

Remove return type for make() #287

merged 3 commits into from
Aug 20, 2024

Conversation

Casmo
Copy link
Contributor

@Casmo Casmo commented Aug 19, 2024

Remove static return type. See #285 (comment)

@lorisleiva
Copy link
Owner

Thank you! 🍺

Two small things before merging:

  • Is there a small regression test we can add here to make sure we don't make that mistake again in the future?
  • @edwinvdpol just checking if you're happy with that change as well.

@Casmo
Copy link
Contributor Author

Casmo commented Aug 19, 2024

Done!

@lorisleiva lorisleiva merged commit e55c1ea into lorisleiva:main Aug 20, 2024
8 checks passed
@lorisleiva
Copy link
Owner

Thanks!

@Casmo Casmo deleted the patch-1 branch August 23, 2024 09:59
@austincarpenter
Copy link

This has broken IDE autocompletion for me. Now PhpStorm thinks handle() when called via ::make()->handle(...) points to vendor/laravel/framework/src/Illuminate/Foundation/Application.php's handle() method.

I deliberately don't use the ::run() syntax for this reason.

@lorisleiva
Copy link
Owner

@austincarpenter Hi there, this was a partial revert of #285 which was only published a patch version earlier (in the matter of days). When did you experience that autocompletion working and then not working?

@austincarpenter
Copy link

@lorisleiva Prior to #285 it was working due to the docblock, also after due to the return type and now post #287 it's not due to neither a docblock nor a return type.

@lorisleiva
Copy link
Owner

Gotcha, so we just need the docblock back right? If so, would you be able to submit a small PR for this and I'll get that merged/patched immediately?

TaylorWilton pushed a commit to TaylorWilton/laravel-actions that referenced this pull request Sep 9, 2024
The make function can't define a return type, because the result could be a Decorator, or the actual action. However, without docblocks or type-hinting, PHPStorm ends up providing garbage autocomplete.
The solution is to instead add back a phpdoc comment for this specific use case.

See lorisleiva#285 and lorisleiva#287 for more details.
TaylorWilton pushed a commit to TaylorWilton/laravel-actions that referenced this pull request Sep 9, 2024
The make function can't define a return type, because the result could be a Decorator, or the actual action.
However, without docblocks or type-hinting, PHPStorm ends up providing incorrect autocomplete information.
The solution is to instead add back a phpdoc comment for this specific use case.

See lorisleiva#285 and lorisleiva#287 for more details.
TaylorWilton pushed a commit to TaylorWilton/laravel-actions that referenced this pull request Sep 9, 2024
The make function can't define a return type, because the result could be a Decorator, or the actual action.
However, without docblocks or type-hinting, PHPStorm ends up providing incorrect autocomplete information.
The solution is to instead add back a phpdoc comment for this specific use case.

See lorisleiva#285 and lorisleiva#287 for more details.
TaylorWilton added a commit to TaylorWilton/laravel-actions that referenced this pull request Sep 9, 2024
The make function can't define a return type, because the result could be a Decorator, or the actual action.
However, without docblocks or type-hinting, PHPStorm ends up providing incorrect autocomplete information.
The solution is to instead add back a phpdoc comment for this specific use case.

See lorisleiva#285 and lorisleiva#287 for more details.
lorisleiva pushed a commit that referenced this pull request Sep 10, 2024
The make function can't define a return type, because the result could be a Decorator, or the actual action.
However, without docblocks or type-hinting, PHPStorm ends up providing incorrect autocomplete information.
The solution is to instead add back a phpdoc comment for this specific use case.

See #285 and #287 for more details.
@austincarpenter
Copy link

Apologies for the delay on this one. Thanks @TaylorWilton @lorisleiva 👏

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.

3 participants