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

Object build by TyphoonFactoryProvider is not honoring <TyphoonComponentFactoryAware> #144

Closed
jasperblues opened this issue Jan 16, 2014 · 6 comments

Comments

@jasperblues
Copy link
Member

No description provided.

@drodriguez
Copy link
Contributor

Ok, so as far as I understand, TyphoonComponentFactoryAware just inject the component factory in the instances after the properties.

There are two cases here:

  • Using withProtocol:dependencies:returns:: I think I will make TyphoonAssistedFactoryBase conform to TyphoonComponentFactoryAware, which will inject the component factory into the factory itself. Then in -[TyphoonAssistedFactoryBase(TyphoonFactoryMethodClosure) forwardInvocation:] I will inject the factory into the instance. I will have to make “internal” some methods of TyphoonComponentFactory(InstanceBuilder) to not repeat code.
  • Using withProtocol:dependencies:factory: and withProtocol:dependencies:factories: are harder, since the instance is built by the user, not us. Capturing those block return values is going to be hard… but maybe possible. It’s going to be slower (has to go through forwardInvocation:).

In case I cannot make it work, I propose relaxing the requisites a little bit: the use inside the block will be the one responsible for injecting the component factory in its object, accesing a property like componentFactory from the first parameter of the block. In my case most of the uses of the factory provider are now using the first method, so I don’t expect this being a hassle for too many people.

I’m going to start working in the first one, and see how far I get for the second one. If anyone sees some problem or thinks I’m going off the rails, please comment.

@ghost ghost assigned drodriguez Jan 31, 2014
drodriguez added a commit to drodriguez/Typhoon that referenced this issue Jan 31, 2014
Types conforming TyphoonComponentFactoryAware passed as the return type for a
factory generated from TyphoonFactoryProvider, will be now correctly injected
with the component factory.

This commit only works when using withProtocol:dependencies:returns:. Others
will come in later commits.

See appsquickly#144
@jasperblues
Copy link
Member Author

Would be very useful to have the former case supported.

In there latter case, where its the config block that explicitly spells out the dependencies, it does actually sound reasonable to ask the user to specify the factory injection with #131 (which I'll implement and push today). What do you think?

drodriguez added a commit to drodriguez/Typhoon that referenced this issue Feb 1, 2014
…s!).

This commit completes the previous one, making factories based on blocks also
inject the factory when appropriate.

The idea is using the already working implementation from the initializer
closures: the block turns into the implementation of a hidden method, the
real method gets intercepted by forwardInvocation, which gets redirected to
the hidden method, and the return value gets injected with the component
factory if needed.

Tests added or updated for the new feature.

See appsquickly#144
@drodriguez
Copy link
Contributor

Well, there you go. Both factories: and returns: works and they get injected automatically the component factory. It makes the blocks go throught the forwardInvocation: path, but that might be actually good (only one path, not two).

Give it a look, a let me know if it covers your use case, and I will merge into master.

@jasperblues
Copy link
Member Author

👍

Very excellent. Let's merge it in.

drodriguez added a commit to drodriguez/Typhoon that referenced this issue Feb 2, 2014
Types conforming TyphoonComponentFactoryAware passed as the return type for a
factory generated from TyphoonFactoryProvider, will be now correctly injected
with the component factory.

This commit only works when using withProtocol:dependencies:returns:. Others
will come in later commits.

See appsquickly#144
drodriguez added a commit to drodriguez/Typhoon that referenced this issue Feb 2, 2014
…s!).

This commit completes the previous one, making factories based on blocks also
inject the factory when appropriate.

The idea is using the already working implementation from the initializer
closures: the block turns into the implementation of a hidden method, the
real method gets intercepted by forwardInvocation, which gets redirected to
the hidden method, and the return value gets injected with the component
factory if needed.

Tests added or updated for the new feature.

See appsquickly#144
@drodriguez
Copy link
Contributor

Done.

I had a strange appearing and disappearing test failure with the cyclic dependency exceptions, but after doing a clean and rebuild they didn’t appear again. I don’t see how I might have introduced such a problem, but if someone sees the problem again, it might be a good thing to investigate if the tests are brittle.

@jasperblues
Copy link
Member Author

Re strange problems: Maybe it was some stale files . . I made a change today that caught an issue about circular deps in initializers - seems we're moving towards raising an exception for that. So I changed the test rules to assert an exception is raised.

I did raise an issue to discuss further though. See it?

(Typed comment on iPhone)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants