search

antonioribeiro / health

Laravel Health Panel
BSD 3-Clause "New" or "Revised" License
1.94k stars 198 forks source link

fix: pass singleton() a closure instead of an object #184

Closed vvanpo closed 4 years ago

vvanpo commented 4 years ago

Laravel 7.22.0 (https://github.com/laravel/framework/releases/tag/v7.22.0) throws TypeErrors when passed an object instead of a closure or class name.

Fixes #183

vvanpo commented 4 years ago

Not being able to run the linter locally is kind of cumbersome, hence the 3 commit amends.

vvanpo commented 4 years ago

@antonioribeiro laravel/framework version 7.22.2 is an important security patch, but this bug is preventing any project using pragmarx/health from upgrading past 7.22.0.

vvanpo commented 4 years ago

The builds appear to be hanging on composer update?

ssmusoke commented 4 years ago

@vvanpo looks like there are some failed builds, can you rerun them

vvanpo commented 4 years ago

@ssmusoke I'm not sure I have permissions to do that. Sorry, I'm not very familiar with Travis-CI, but I can't seem to find any options that would let me rerun the build.

antonioribeiro commented 4 years ago

Thanks for this. Just fixed tests and tagged a new version. I just don't get why are we having a breaking change in Laravel in a minor version...

ssmusoke commented 4 years ago

@antonioribeiro seems like it was tightening of type usages which lead to the break

vvanpo commented 4 years ago

Yeah, it's an unfortunate demonstration of Hyrum's Law.

Technically the documentation for Container::bind() was always explicit in requiring a closure or a string: https://laravel.com/api/7.x/Illuminate/Contracts/Container/Container.html#method_bind

But the fact that passing an object worked just fine meant that people used it. The Laravel maintainers should have known this was going to break things downstream.

ssmusoke commented 4 years ago

@antonioribeiro You tagged and released 0.10.2 which already existed from October 2019, the new version needs to be 0.10.3 and the new tag deleted too which may break deploys

antonioribeiro commented 4 years ago

It's odd because I didn't forced the tag push, but it still overrode it? Ok I just pushed the new 0.10.3 and deleted 0.10.2.

ssmusoke commented 4 years ago

Awesome the tag now works

I think the issue was the capitalization of the v, one was capital the other was lower case so *nix style both were accepted