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

[11.x] fix: allows injection using multiple interfaces with the same concrete implementation #53275

Merged
merged 20 commits into from
Dec 14, 2024

Conversation

jamiethorpe
Copy link
Contributor

Fixes issue 53110

This allows multiple interfaces to be injected when they use the same concrete implementation. This change also prevents injecting the same interface twice, but I'm not sure if that's necessary.

@jamiethorpe
Copy link
Contributor Author

Whoops, missed some failing tests. I'll get these fixed up.

@taylorotwell taylorotwell marked this pull request as draft October 23, 2024 13:01
@jamiethorpe jamiethorpe marked this pull request as ready for review November 1, 2024 00:38
@taylorotwell
Copy link
Member

Seems like we would want a test for this new behavior I think.

@taylorotwell taylorotwell marked this pull request as draft November 11, 2024 21:32
@jamiethorpe jamiethorpe marked this pull request as ready for review November 14, 2024 11:41
@jamiethorpe
Copy link
Contributor Author

Should be good now. I wasn't 100% sure if the test fit into any of the existing test files, so I added a new one.

@taylorotwell taylorotwell merged commit a306193 into laravel:11.x Dec 14, 2024
38 checks passed
@taylorotwell
Copy link
Member

Sorry for delay - thanks. 👍

@jasonvarga
Copy link
Contributor

This caused an issue with route model binding. See: #53952

@jamiethorpe since you authored this PR, maybe you could give some insight? It would be appreciated! 🙏

@taylorotwell
Copy link
Member

Reverting for now.

@jasonvarga
Copy link
Contributor

Thank you

@jamiethorpe
Copy link
Contributor Author

@jasonvarga This looks like an oversight on my part. I probably won't get to it until after the holidays, but I'll take another stab at it and and tests to make sure the refactor doesn't break the same functionality. Thanks for finding that 😬

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.

Multiple interfaces injection fails if they have the same concrete class
3 participants