Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Silently resolve an item from IOC so that the resolving callbacks wont fire #1470

Open
imanghafoori1 opened this issue Jan 4, 2019 · 8 comments

Comments

@imanghafoori1
Copy link

Currently if you register some resolving (or resolved) callbacks for an abstract, you have no way to suppress them when you use app()->make(...

Maybe a new method app()->makeSilently(... would do the job.

@mfn
Copy link

mfn commented Jan 4, 2019

suppress them

Sorry I'm totally in the dark what you mean with "suppress" here and what you want to suppress. Can you elaborate on the use case?

@imanghafoori1
Copy link
Author

Simply you want to resolve the object but you want to skip the callbacks from happening.

app()->resolving('foo', function ( ) { var_dump('I am fired'); });

app()->make('foo');

@Patryk27
Copy link

Patryk27 commented Jan 4, 2019

What would be the point of explicitly registering a callback and then not firing it?
Just don't register the callback.

@sisve
Copy link

sisve commented Jan 4, 2019

I would use this to create instances of form requests. I have many cases where I want to manually create a form request instances just to retrieve the validation rules, so that I can then render relevant validation attributes in a html form. The problem with the current logic is that Laravel automatically validates the resolved instances, which throws an validation exception, when I am creating them. (This applies to everything implementing the ValidatesWhenResolved interface.)

@Patryk27 So, the case for me is that I am not the one that registered the callback. I need to suppress someone else's callback.

https://github.com/laravel/framework/blob/9155faa8264515171ddaa528f375be09fadeb055/src/Illuminate/Foundation/Providers/FormRequestServiceProvider.php#L29-L37

@Patryk27
Copy link

Patryk27 commented Jan 7, 2019

@sisve: Why you're creating a FormRequest through the container? Looks like new MyFormRequest() should do just fine, unless you're overwriting the default (Symfony's) constructor, which seems unsound.

Having that said, I'm not keen on creating some mechanisms (the resolving() method) and then some other mechanisms to avoid them (that hypothetical makeSilent() thingy) - it's kinda like the CSS !important hack, which is exactly this: a hack, because you failed to structure your application correctly.

I understand there might be some useful cases for this (especially regarding 3rd party code - otherwise this thread would've never appeared), but I think there might be better solutions too (in terms of the application's design, not the framework itself) - similarly how the !important rule is not strictly necessary and causes a great deal of harm in larger codebases.

@sisve
Copy link

sisve commented Jan 7, 2019

@Patryk27 Some requests are using constructor injection. You can currently override the form requests __construct to your hearts desire; the FormRequestServiceProvider's resolving callback will overwrite/initialize the request no matter what you've done previously. You don't have to keep all those existing constructor parameters, the default values are good-enough since they are overwritten afterwards.

I agree that suppressing the resolving callback is a weird hack. My use-case is very specific, and I would perhaps be better suited with a change to how form requests specifically are instantiated, instead of modifying the ioc container for this case.

class EducationFormRequest extends Request {

    /** @var EducationRepository */
    private $educationRepository;

    public function __construct(EducationRepository $educationRepository) {
        parent::__construct(); // The default values here doesn't matter.

        $this->educationRepository = $educationRepository;
    }

    public function withValidator(Validator $validator) {
        $validator->after(function (Validator $validator) {
            $this->validateEducationData($validator);
        });
    }
    
    public function validateEducationData(Validator $validator) {
        // ...
    }

}

@Patryk27
Copy link

Patryk27 commented Jan 7, 2019

There's no need to override constructor in your case - you can as well just do:

/** @var EducationRepository */
$educationRepository = $this->container->get(EducationRepository::class);

And bam - no more problems.

The actual framework-side solution for this one would be to get rid of the ugly hierarchy (form request -> laravel request -> symfony request) and just utilize composition, but that'd be a pretty big change.

@imanghafoori1
Copy link
Author

Let me mention that laravel internally need this feature to solve a problem of the IOC.
laravel/framework#27066

here I used this to remedy the extra calls to resolving callback.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants