fix: Lazily bind dependencies #2321
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 7, 2023
|
What about implementing a test to cover the last issues related? In time, I will test these modifications locally and let you know about the results EDIT: It worked here |
|
@olivernybroe thanks for the update! What are your thoughts on test suite changes to support it? |
|
@drbyte I can't seem to find a great way of doing that, as we currently resolve all the classes in the setup method in the So when we get to this part to check the lazy binding, it doesn't matter as they are resolved. |
|
Agreed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Here is a new version of the lazy binding solution. The reason why it was causing issues the first time around was because the
registerPermissionsmethod was called oncePermissionRegistrarwas resolved instead of whenGatewas resolved.I tested this out in a fresh installation of Laravel with the
candirective in a file and my change here worked. but ofc. would love someone else to also validateping @drbyte