added PresentersExtension WIP

[dg]

No description provided.

[vojtech-dobes]

If I am correct, Assert::count(... can be used here.

[dg]

Wow, we have assert::count!

[vojtech-dobes]

I am addicted to new Nette features...

[JanTvrdik]

?
Should this force rebuilding the container?

[dg]

There should be some legal way how to invalidate container.

[JanTvrdik]

Yes, there should be. But you certainly must not do this in production. If someone is using presenters without the Presenter suffix, this will get triggered on every request. Or am I wrong?

[dg]

I think that it does nothing on production nor on development, it is just WIP.

[JanTvrdik]

Pro tip: if you know that code has flaws, add a comment such as "// TODO: this may kill performance if application uses presenter not discovered by the extension".

[dg]

👍

(but it really does nothing, it deletes unused .meta file)

[xificurk]

Great, I'm really excited to see this in Nette!
I think the extension should somehow resolve situations like this:

class APresenter extends Nette\Application\UI\Presenter {}
class BPresenter extends APresenter {}

Depending on the order in which the classes are processed, you'll get one or both presenters registered.
I know this should probably be fixed in the app code, but I would at least expect some kind of exception alerting me to a possible problem.
(Note: I actually sometimes use this - ErrorPresenter in a module inherits from a general ErrorPresenter that can work on its own.)

[dg]

@xificurk fixed

[dg]

I tell myself that ApplicationExtension and PresentersExtension should be merged.

[xificurk]

This should search by tag as well.
And before processing found classes, there should be one more pass over already defined presenter services, that would add appropriate tags to them.

[dg]

That's exactly what it does.

[xificurk]

Nope...
a) It goes over the list compiled by findClasses(), thus ignoring presenters manually registered as services by user.
b) There is still the problem I mentioned earlier - if findClasses() returns the list containing BPresenter before APresenter, you'll get only BPresenter registered with a wrong tag nette.presenter=APresenter. Because first BPresenter is registered and then when the loop later gets to processing APresenter, the findByType("APresenter", FALSE) will return previously registered definition of BPresenter and set it wrong tag.

[dg]

Now it's probably ok

[xificurk]

What do you think about something like this xificurk/nette-application@d6c90a7 ?

[dg]

What is it good for?

[xificurk]

@dg Currently it's really hard to use presenters distributed in a module with some package, because you cannot easily customize (templates, extra components, ...) it to the needs of your specific application.
xificurk/nette-application@d6c90a7 allows you to manually register presenter and set its tag in such a way, that it would "replace" the original.

[dg]

You can manually register presenter https://github.com/dg/nette-application/blob/extension/tests/Application.DI/PresentersExtension.basic.phpt#L57

[xificurk]

That's not the point. Let's say you've installed some package that ships with several presenter classes FooPackage\FooModule\*Presenter, it has registered a proper presenter mapping, so you refer to e.g. FooPackage\FooModule\BarPresenter as Foo:Bar. Now if you need to change one teeny tiny detail in this presenter and leave the rest, how do you do it?
I would like simply overriding that presenter in my app configuration like:

services:
    -
      class: MyBarPresenter
      tags: {'nette.presenter': 'FooPackage\FooModule\BarPresenter'}

[dg]

Now if you need to change one teeny tiny detail in this presenter and leave the rest, how do you do it?

Just as in Nette 2.1:

services:
    -
      class: FooPackage\FooModule\BarPresenter
      setup: ...

Tag nette.presenter is internal thing.

[xificurk]

Yeh, but that presenter is supplied by 3rd party package, e.g. same as NetteModule, you can't directly touch it.

[dg]

I do not understand you at all. Do you have an example?

[xificurk]

@dg To be specific - we've got a composer package that ships with Nette module providing generic web administration interface, but sometimes we need to customize some minor stuff, so app-specific presenter is created like:

MyFooPresenter extends GenericPackage\AdminModule\FooPresenter {
//changes to a generic implementation
}

So, we need to somehow force the application to use everywhere MyFooPresenter instead of the generic version shipped with the package.

But as I think about it now, it will be probably better for us to rewrite presenter mapping part of PresenterFactory.

[dg]

I see. And agree that it should be probably solved mapping.

[dg]

What about this: when you define setup for base presenter, it will be used by all children.

[fprochazka]

@dg a bit magical, but it makes sense. But reset of setups for "child" service should also work.

[dg]

Presenters should be probably defined inside presenters instead of services.

They are not services. And abstract classes cannot be used as services.

[fprochazka]

Sure, but you've made some good points here. Having some presenters as service should keep working, right?

[JanTvrdik]

They are not services.

Curiosity question: What prevents them from being services? You would just need to make sure that the internal state is cleared after or before run(), or not?

[fprochazka]

@JanTvrdik that is unneccesary... having them registered as services, but never really fetching the instance from container, but rather using it as a factory is less magic and works great.

---

Having some presenters as service should keep working, right?

I didn't mean having their instances shared, but rather having them only registered as services and using the createService every time, exactly as PresenterFactory is doing it now.