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

Adding Facade For Calling The Functions #161

Closed
wants to merge 7 commits into from

Conversation

TarsisioXavier
Copy link
Contributor

The Feature 📦

In Laravel facades are a common thing to have, so I've made this small class which calls the functions of the helper file via static calls.

This PR adds a convenient way to call the functions without importing every function the dev uses in his script. I’ve also added a way to inject this facade class as an object - if the dev desires - and call the functions using “->”.

Possible Usages 📝

  • Can be injected in a class or method like a controller for example.
  • Doesn't need to import all functions used in the script.
Before After
Before After

@TarsisioXavier TarsisioXavier marked this pull request as draft September 9, 2024 22:39
@TarsisioXavier TarsisioXavier marked this pull request as ready for review September 9, 2024 23:23
@devajmeireles
Copy link
Contributor

Genius! But wouldn't this PR be aimed at adding only the Facade? I mean, maybe you don't need things as examples, after all it's the same as the same.

@jessarcher
Copy link
Member

jessarcher commented Sep 10, 2024

Hey @TarsisioXavier,

If you want to reduce the number of imports, you could import the Laravel\Prompts namespace and reference the functions as follows:

<?php

use Laravel\Prompts;

$name = Prompts\text('What is your name?');

Also, I'd avoid calling the class you've created a facade because it doesn't extend Laravel's Facade class so it doesn't have the facade features that the name implies.

@devajmeireles
Copy link
Contributor

Hey @TarsisioXavier,

If you want to reduce the number of imports, you could import the Laravel\Prompts namespace and reference the functions as follows:

<?php

use Laravel\Prompts;

$name = Prompts\text('What is your name?');

Also, I'd avoid calling the class you've created a facade because it doesn't extend Laravel's Facade class so it doesn't have the facade features that the name implies.

Do you agree with the idea to have a Facade for this?

@jessarcher
Copy link
Member

Do you agree with the idea to have a Facade for this?

I don't think I see a real need for the static class in this PR, especially with the namespace import already solving the multiple import issue mentioned in the PR description.

Having it as a real facade is potentially interesting for mocking prompts, but it's not something I've personally felt the need for. Additionally, we're trying to avoid depending on any illuminate/* packages in Prompts because we currently have a circular dependency with laravel/framework that is causing some grief each time we do a major release of Laravel.

@TarsisioXavier
Copy link
Contributor Author

Genius! But wouldn't this PR be aimed at adding only the Facade? I mean, maybe you don't need things as examples, after all it's the same as the same.

Thanks for the compliment @devajmeireles. Yeah those files wore only for me to verify if everything behave the same way, I only wanted to be sure, so they are completely disposable.

@TarsisioXavier
Copy link
Contributor Author

Good morning @jessarcher,

I did notice that you guys didn't make any facade and I was wondering why, so I tried my best to create one without the illuminate/support dependency. 😅
Even though it doesn't extend from Laravel facade class it follows the design pattern of a facade forwarding the function call to the "real" function. 🙂
Since there is no intention to add a facade for this package I guess we can close this PR then? 🥲

@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

@TarsisioXavier TarsisioXavier deleted the feature/facade branch September 10, 2024 22:53
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.

4 participants