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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.8] Bug fix related to extra calls to resolving callbacks (The other way) #27066

Merged

Conversation

imanghafoori1
Copy link
Contributor

@imanghafoori1 imanghafoori1 commented Jan 4, 2019

(Following the currently open PR #27014) This is an other way of fixing the issue of "multiple calls to resolving callback" which came to my mind while I was under the shower today.馃毧 馃泙

Compared to the solution in: #27014 ,it requires a less code and is more understandable and logical.
Plus, it adds an opportunity to have a new feature to the IOC container for "resolving an object silently". (Which will suppress the resolving callbacks.)
for example : makeSilent(...

This way we fire the callback attached to the interface and skip the re-firing it when resolving the implementation.
which makes more sense, because the callback was actually attached to the interface like this :
app()->bind(someInterface::class, 'Myclass'); `

Anyway I do not know which one you like most, so I leave them both open.

@GrahamCampbell
Copy link
Member

Should this not target 5.7?

@imanghafoori1
Copy link
Contributor Author

It was on 5.7 but closed.

Since it is not considered to be 100% backward compatible.
it changes the signature of resolve method.

@GrahamCampbell
Copy link
Member

Oh, the PR title is misleading, as it says add missing tests, but the PR actually changes the implementation too.

@imanghafoori1 imanghafoori1 changed the title [5.8] Add missing tests for the resolving callbacks on the container [5.8] Fix for multiple calls to resolving callbacks issue + Add missing tests Jan 4, 2019
@imanghafoori1 imanghafoori1 changed the title [5.8] Fix for multiple calls to resolving callbacks issue + Add missing tests [5.8] Bug fix related to extra calls to resolving callbacks (The other way) Jan 4, 2019
+ bug fix related to multiple calls to "resolving" and "afterResolving" callbacks
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.

None yet

3 participants