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

[5.4] Container: Add a new method to remove extenders of abstract #19269

Merged
merged 2 commits into from
May 19, 2017
Merged

[5.4] Container: Add a new method to remove extenders of abstract #19269

merged 2 commits into from
May 19, 2017

Conversation

LeoYang90
Copy link
Contributor

@LeoYang90 LeoYang90 commented May 19, 2017

public function testExtendAfterResolve()
{
    $container = new Container;
    $container->bind('foo', function () {
        $obj = new StdClass;
        $obj->foo = 'bar';

        return $obj;
    });

    $container->extend('foo', function ($obj, $container) {
        $obj->bar = 'baz';

        return $obj;
    });

    unset($container['foo']);

    $container->bind('foo', function () {
        return 'foo';
    });

    $obj = $container->make('foo');
}

It will throw a exception 'Attempt to assign property of non-object'.
If we want to rebind the abstract that has been changed by extend(), we can not clear the $extenders[]. So we can never rebind 'foo' to a closure that returns string likes 'foo'.

@LeoYang90 LeoYang90 closed this May 19, 2017
@LeoYang90 LeoYang90 reopened this May 19, 2017
@taylorotwell taylorotwell merged commit bd08582 into laravel:5.4 May 19, 2017
{
$abstract = $this->getAlias($abstract);

if (isset($this->extenders[$abstract])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement is unnecessary, unset will do the check automatically

@LeoYang90 LeoYang90 deleted the featUnsetExtend branch May 20, 2017 07:35
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