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

Add more tests for Container #637

Conversation

girardinsamuel
Copy link
Contributor

  • Add more tests to Container
  • Show the issue with the namespace. Instead of raising an error that the binding cannot be found an unhandled error is raised.

This also outlines that the Container is considering differently
src.masonite.response.Response and masonite.response.Response.

@girardinsamuel
Copy link
Contributor Author

girardinsamuel commented May 19, 2022

If you look closely, it fails when trying to resolve the nested dependency (Application) passed in the init of the SomeStringTypeHintedAppObject. When type hinted with a string it causes an issue because when resolving the dependency it won't be able to find the class associated to "Application".

Although Response class is written the same way, as it is bound explicitly, the container never have to instantiate again on the fly and resolve this application parameter. That is why it's working with Response class and not with SomeStringTypeHintedAppObject class.

So currently, the "resolving on the fly" feature does not handle the string type hinted parameters.

@girardinsamuel
Copy link
Contributor Author

Looks good to be merged !

@josephmancuso josephmancuso merged commit 5d5b6eb into MasoniteFramework:4.0 May 22, 2022
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

2 participants